Combine linq statement or leave











up vote
-2
down vote

favorite












I have the following code. Should the linq statements be combined or left as is?
I typically like to break it up but some members on my team like a one-liner.



Thoughts?



        var startDate = DateTime.UtcNow;

        var endDate = startDate.AddDays(expirationDays);



        var allTokens = await repository.Query<Domain.Models.AccessToken>()

            .Where(x => !x.IsRevoked)

            .ToListAsync()

            .ConfigureAwait(false);



        var filteredTokens = allTokens.GroupBy(x => x.ApplicationName)

            .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

            .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate);



        return filteredTokens.Select(x => x.Id).ToList();





c# linq







share|improve this question






















  • ha, that's completely up to you. Kind of hard to answer that question.
    – ylax
    Nov 19 at 21:32










  • The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
    – juharr
    Nov 19 at 21:32






  • 1




    Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
    – Flydog57
    Nov 19 at 21:41















up vote
-2
down vote

favorite












I have the following code. Should the linq statements be combined or left as is?
I typically like to break it up but some members on my team like a one-liner.



Thoughts?



        var startDate = DateTime.UtcNow;

        var endDate = startDate.AddDays(expirationDays);



        var allTokens = await repository.Query<Domain.Models.AccessToken>()

            .Where(x => !x.IsRevoked)

            .ToListAsync()

            .ConfigureAwait(false);



        var filteredTokens = allTokens.GroupBy(x => x.ApplicationName)

            .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

            .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate);



        return filteredTokens.Select(x => x.Id).ToList();





c# linq







share|improve this question






















  • ha, that's completely up to you. Kind of hard to answer that question.
    – ylax
    Nov 19 at 21:32










  • The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
    – juharr
    Nov 19 at 21:32






  • 1




    Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
    – Flydog57
    Nov 19 at 21:41













up vote
-2
down vote

favorite









up vote
-2
down vote

favorite











I have the following code. Should the linq statements be combined or left as is?
I typically like to break it up but some members on my team like a one-liner.



Thoughts?



        var startDate = DateTime.UtcNow;

        var endDate = startDate.AddDays(expirationDays);



        var allTokens = await repository.Query<Domain.Models.AccessToken>()

            .Where(x => !x.IsRevoked)

            .ToListAsync()

            .ConfigureAwait(false);



        var filteredTokens = allTokens.GroupBy(x => x.ApplicationName)

            .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

            .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate);



        return filteredTokens.Select(x => x.Id).ToList();





c# linq







share|improve this question













I have the following code. Should the linq statements be combined or left as is?
I typically like to break it up but some members on my team like a one-liner.



Thoughts?



        var startDate = DateTime.UtcNow;

        var endDate = startDate.AddDays(expirationDays);



        var allTokens = await repository.Query<Domain.Models.AccessToken>()

            .Where(x => !x.IsRevoked)

            .ToListAsync()

            .ConfigureAwait(false);



        var filteredTokens = allTokens.GroupBy(x => x.ApplicationName)

            .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

            .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate);



        return filteredTokens.Select(x => x.Id).ToList();





c# linq




c# linq






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked Nov 19 at 21:17









Jason Jay

325




325












  • ha, that's completely up to you. Kind of hard to answer that question.
    – ylax
    Nov 19 at 21:32










  • The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
    – juharr
    Nov 19 at 21:32






  • 1




    Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
    – Flydog57
    Nov 19 at 21:41


















  • ha, that's completely up to you. Kind of hard to answer that question.
    – ylax
    Nov 19 at 21:32










  • The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
    – juharr
    Nov 19 at 21:32






  • 1




    Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
    – Flydog57
    Nov 19 at 21:41
















ha, that's completely up to you. Kind of hard to answer that question.
– ylax
Nov 19 at 21:32




ha, that's completely up to you. Kind of hard to answer that question.
– ylax
Nov 19 at 21:32












The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
– juharr
Nov 19 at 21:32




The main difference here is how much data you end up pulling from the DB and filtering in memory instead of letting the DB handle for you.
– juharr
Nov 19 at 21:32




1




1




Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
– Flydog57
Nov 19 at 21:41




Do you need allTokens to be a list? Is it a Queryable without the ToListAsync call? If so, then, if you remove the ToListAsync call, doing it all in one go or as three separate statements is purely a matter of taste.
– Flydog57
Nov 19 at 21:41












1 Answer
1






active

oldest

votes

















up vote
2
down vote



accepted










Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.



So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:



var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;


... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)



Trying to read what your coworkers would have:



var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();


... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.






share|improve this answer

















  • 1




    This also makes it easy to view intermediate results when debugging.
    – TrueWill
    Nov 19 at 22:20






  • 1




    @TrueWill - very very good point. :-)
    – Kevin
    Nov 19 at 22:22










  • agreed, thank you
    – Jason Jay
    Nov 19 at 22:25











Your Answer






StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53382763%2fcombine-linq-statement-or-leave%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























1 Answer
1






active

oldest

votes








1 Answer
1






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
2
down vote



accepted










Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.



So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:



var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;


... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)



Trying to read what your coworkers would have:



var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();


... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.






share|improve this answer

















  • 1




    This also makes it easy to view intermediate results when debugging.
    – TrueWill
    Nov 19 at 22:20






  • 1




    @TrueWill - very very good point. :-)
    – Kevin
    Nov 19 at 22:22










  • agreed, thank you
    – Jason Jay
    Nov 19 at 22:25















up vote
2
down vote



accepted










Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.



So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:



var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;


... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)



Trying to read what your coworkers would have:



var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();


... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.






share|improve this answer

















  • 1




    This also makes it easy to view intermediate results when debugging.
    – TrueWill
    Nov 19 at 22:20






  • 1




    @TrueWill - very very good point. :-)
    – Kevin
    Nov 19 at 22:22










  • agreed, thank you
    – Jason Jay
    Nov 19 at 22:25













up vote
2
down vote



accepted







up vote
2
down vote



accepted






Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.



So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:



var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;


... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)



Trying to read what your coworkers would have:



var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();


... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.






share|improve this answer












Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.



So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:



var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;


... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)



Trying to read what your coworkers would have:



var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();


... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.







share|improve this answer












share|improve this answer



share|improve this answer










answered Nov 19 at 22:16









Kevin

1,589418




1,589418








  • 1




    This also makes it easy to view intermediate results when debugging.
    – TrueWill
    Nov 19 at 22:20






  • 1




    @TrueWill - very very good point. :-)
    – Kevin
    Nov 19 at 22:22










  • agreed, thank you
    – Jason Jay
    Nov 19 at 22:25














  • 1




    This also makes it easy to view intermediate results when debugging.
    – TrueWill
    Nov 19 at 22:20






  • 1




    @TrueWill - very very good point. :-)
    – Kevin
    Nov 19 at 22:22










  • agreed, thank you
    – Jason Jay
    Nov 19 at 22:25








1




1




This also makes it easy to view intermediate results when debugging.
– TrueWill
Nov 19 at 22:20




This also makes it easy to view intermediate results when debugging.
– TrueWill
Nov 19 at 22:20




1




1




@TrueWill - very very good point. :-)
– Kevin
Nov 19 at 22:22




@TrueWill - very very good point. :-)
– Kevin
Nov 19 at 22:22












agreed, thank you
– Jason Jay
Nov 19 at 22:25




agreed, thank you
– Jason Jay
Nov 19 at 22:25


















draft saved

draft discarded




















































Thanks for contributing an answer to Stack Overflow!


  • Please be sure to answer the question. Provide details and share your research!

But avoid …



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.





Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


Please pay close attention to the following guidance:


  • Please be sure to answer the question. Provide details and share your research!

But avoid …



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53382763%2fcombine-linq-statement-or-leave%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







-

Popular posts from this blog

Costa Masnaga

Image
.mw-parser-output .avviso .mbox-text-div>div,.mw-parser-output .avviso .mbox-text-full-div>div{font-size:90%}.mw-parser-output .avviso .mbox-image div{width:52px}.mw-parser-output .avviso .mbox-text-full-div .hide-when-compact{display:block} Questa voce o sezione sull'argomento Lombardia non cita le fonti necessarie o quelle presenti sono insufficienti . Puoi migliorare questa voce aggiungendo citazioni da fonti attendibili secondo le linee guida sull'uso delle fonti. Segui i suggerimenti del progetto di riferimento. Costa Masnaga comune Localizzazione Stato   Italia Regione Lombardia Provincia Lecco Amministrazione Sindaco Sabina Panzeri (Al centro il cittadino) dall'08/06/2009 Territorio Coordinate 45°46′N 9°17′E  /  45.766667°N 9.283333°E 45.766667; 9.283333  ( Costa Masnaga ) Coordinate: 45°46′N 9°17′E  /  45.766667°N 9.283333°E 45.766667; 9.283333  ( Costa Masnaga ) Altitudine 318 m s.l.m...
Read more

Fotorealismo

Image
Guerrino Boatto, Venice's St. Barnaba Church and canal Il fotorealismo è un genere di pittura basato sull'uso di una o più fotografie dalle quali l'artista prende le informazioni necessarie da utilizzare poi nel processo della creazione dell'opera, in genere ad olio o ad acrilico, al fine di avere un aspetto finale il più simile possibile alla fotografia. Indice 1 Storia 2 Lista dei fotorealisti 3 Note 4 Voci correlate 5 Altri progetti Storia | Il fotorealismo è un movimento che nasce negli Stati Uniti d'America alla fine degli anni sessanta [1] sulla scia della Pop Art [2] [3] [4] , e insieme condividono il carattere reazionario verso la travolgente ascesa dei media, che alla metà del ventesimo secolo si trasforma in un fenomeno di massa talmente imponente da minacciare il valore dell'immagine nell'arte. [5] [6] Ne consegue così che, colto il gesto d'inizio che fu della Pop Art, i fotorealisti aprono ad un ...
Read more

Sidney Franklin

Image
Questa voce sull'argomento registi statunitensi è solo un abbozzo . Contribuisci a migliorarla secondo le convenzioni di Wikipedia. Sidney Franklin, foto di Carl Van Vechten Oscar alla memoria Irving G. Thalberg 1943 Sidney Franklin , all'anagrafe Sidney Arnold Franklin , (San Francisco, 21 marzo 1893 – Santa Monica, 18 maggio 1972), è stato un regista, produttore cinematografico e attore statunitense. Il 20 giugno 1963, si sposò con l'attrice australiana Enid Bennett. Il matrimonio durò fino alla morte dell'attrice, il 14 maggio 1969 [1] Indice 1 Filmografia 1.1 Regista 1.2 Aiuto regia (parziale) 1.3 Produttore 1.4 Attore (parziale) 2 Note 3 Altri progetti 4 Collegamenti esterni Filmografia | Regista | The Baby , co-regia Chester M. Franklin (1915) The Rivals , co-regia Chester M. Franklin (1915) Little Dick's First Case , co-regia Chester M. Franklin (1915) Her Filmland Hero , co-...
Read more