The worst about this code is not using the data.Count to initialize the capacity. Currently the List initializes with a capacity of 4, doubling each time it requires more space. So lets assume the input parameter has 300 entries: 4,8,16,32,64,128,256,512 -> 8 arrays. Unnecessary allocation, unnecessary copies, unnecessary wasted memory.
You can write the original code without any ifs or conditional operators return data?.Select(d => d * 2).ToList() for the code that returns an empty list when null just slap a "?? []" on the end
I agree with so many of your points, such as don't return null, return empty. Return early, Avoid else, elegant use of LINQ. I also like the way you stepped through reducing the code, I think so many people get themselves in trouble when they don't do it iteratively.
Golden Rule: accept the _minimum_ you require, return the _maximum_ you produce. If the only thing you do in your method is to enumerate through a collection, then the type of the parameter should be IEnumerable.
IList or ICollection IMO. I used to always default to IEnumerable as well but you don't necessarily know what the implementation is and it could be not safe or efficient to enumerate it multiple times and you wouldn't know if you're the only method enumerating it
@@dennycrane2938 Hmm... What do you mean you don't know what the implementation is? You ARE the author of the method! It does not matter if you need to enumerate multiple times. As I stated, if the _only_ thing you do is enumerating through the collection, then IEnumerable is enough - that makes the method the most versatile. You're aware of polymorphism, right?
+1 for inverting ifs, more people need to get on that bandwagon. It's cleaner, easier to read, reduces nasty nesting, etc. The only thing I do differently is I do put EF extension methods on new lines, I feel it makes it better for developers to read the steps that linq is going through against data.
I sometime puts linq on own rows but it depends, simple .ToList or similar I usually leave at the end since they rarely are the important part of the line. And if I only have one main linq that IS the meat of the row and not to long I also usually do not split the line, split lines also add some extra work for the brain to join together. But for any longer statement or when I have multiple important linq I also tend to have each on a new line stating with the . to also highlight that its a continuation and not a new statement. Similar to how I do with ternary when I do use them, I put the ? and : as new rows since that clearly indicate that the line is an extension of the former row and seeing one line with ? and the next with :, at least for me very much screams ternary. But I rarely use ternary now a day unless in simple cases where the result is readable enough that saving the extra lines is worth it. Its always "which one reads the easiest".
By simple stretching the code from 3 lines to 4 lines the linked in author could have made the neste ternary operator version so much more readable public List ProcessData(List data) => data == null ? null : data.Count == 0 ? new List() : data.Select(d => d * 2).ToList(); This way, it is always condition on the left, "early return" on the right.
That's a legit shocker. Someone who has fallen for the "fewer lines always = better" lie. The compiler doesn't care ... but the next developer who has to read and try to understand the code sure does - you're making it nice for THEM. And THEM might be YOU, 5 years from now, when you'll appreciate clarity.
The problem screamed elvis operator to me, but it's also a silly toy example where a null input returns null, so your code is wrong. I would say the design of returning null instead of an empty list is a design problem but that's the toy example as it exists.
@@AldoInza Your use of the extra`?` leads to more boxing/unboxing, I feel the code @DimitriKandiner submitted is more efficient and arguably more readable.
@@jasonpirok but it can never return null. I would probably say fix the consumer so it doesn't need a null return, but maybe it's some odd db column or a config parser where null means inherit value etc.
Only thing I usually do different from your solution, is I one-line the if-return (when it's a simple if-statement). Another option is to use the conditional operator but on multiple lines. That also reads very nicely.
Nullable reference types do NOT prevent you from passing null at runtime - it's compiler air-ware. So there is nothing wrong with null checks in the outer perimeter / public surface.
Dev: Should i do if-else or early-return or ternary-ternary-ternary? Ternary is better right? Fewer lines, faster code? Compiler: I don't give a shit. JIT Compiler: I may even make it the same, lol. CPU: I may change the order of operations, lol. Dev#2: WTF? Who wrote that shit?
Ternary operators are not the easiest to read when composed. Never be afraid to add a few more lines of code, if that makes the code easier to read. I often see complicated one-liners with ternary operators from the 'sophisticated' developer, proud that he resolved the problem in one line of code. Would much rather look at 5-10 lines of code that are obvious and easy to understand in an instant, than having to waste a minute of time, mental energy following the sophisticated one-liner logic. That's one difference between experienced vs intermediate/junior developers.
So true. If you really want to reduce the size of a function, break parts out into methods with good names so that you in most cases would not even need to see the inner code to understand what it does
that example most bad reason is not title said, when process sequence input or output, 1. null is no means (1-has element, 2-empty, or 3-throw when error , can explain all case about sequence) 2. no need to check element exist before iterator (because no element will run 0 times) then that will be follow => data.Select(x=> x * 2); i may not use fast return to explain it
4 години тому
The Select() call introduces a whole slew of allocations you don't need, like rebuilding a list from scratch without pre-setting a capacity. Instead you should use ConvertAll(d => d * 2).
Ternary operators are awful for readability, especially in C, let alone C#. I like your solution as it's better for readability and maintenance. I always validate the inputs and return early as it makes debugging a lot easier.
I think this would be a reasonable way of doing it: public List ProcessData(List data) => data is null ? null // Only returning null to be compatible with OP's example, I agree pattern matching would be better. : data.Count is 0 ? [] : data.Select(x => x * 2).ToList(); Tho I agree if statements can be more readable. Personally, I would also not make the method accept a List and return a new List. Generally speaking, if I take a mutable collection, then I would mutate it rater than returning a new one. If I don't want to mutate the collection, then I would take a read only abstraction instead.
Ha, they got me.... a code cop about how you should always return early, being a bad thing... I was just about ready to explode. Yeah their code for what 'you should do' is hot garbage. When will people learn that LoC isn't at all better for readability, performance, ... nor is it 'a liability for the company', which I hear all the time... CRAPY CODE hurts all of those things... good code is actually readable, performant and actually isn't a liability.... and sometimes it takes 5x the amount of lines to do it, and that's 100% okay.
I feel that code should always tell a story and should be just as reasonable. If I look at a function and it is either 50 lines long or is completely unreadable it goes on the backlog as tech debt or I get permission to refactor it immediately. The primary reason for this is that in either situation, the single use principal is violated which indicates the function was not written with intent. I have told devs for years that the `if else` statement that has a return in the else needs to be redone because nesting is bad and that is the primary cause.
Im 100% with you on returning empty collections if immutable. But I've always wondered about excessive allocation when returning empty for mutable collections, especially if an empty result will be unused.
@@nickchapsas But List is mutable and will allocate. Personally I'm okay returning List? For private code to avoid the allocation. I probably wouldn't do it in public code. But then I rarely return new Lists in public code anyway. But I don't think sometimes returning an empty list and sometimes returning null (like the example) makes sense in any scenario.
even with nullable parameter, it's just a questionmark away; data?.Select(x => x * 2).ToList() Or if you don't want null return; [.. data?.Select(x => x * 2) ?? []]
A parameter can still be null, even when declared non-nullable, since it might be consumed by something that's not nullable-aware or some silly programmer passes null! . You should always handle the possibility that a parameter could be null in public/protected code.
@@RobinHood70 a programmer not respecting nullability will experience a crash that they need to fix on their side, input sanitation is not necessary on every level in the code
@@Flimzes I 100% disagree with that unless the code is private/internal. As I said, though, it's not just a question of a programmer respecting nullability...not everything is nullability-aware. When they introduced the nullability options, they explicitly said "don't stop checking for null". As a practical example, look at Microsoft's code. It all checks for null in all public-facing methods, even when something's declared non-nullable.
I've had situations where switch is not applicable and I'd have to chain if-else if-else based on multiple variables. IMO they can be readable if formatted properly (even nested) but that's really a scenario for a kind of switch expression we don't have yet in c#.
Hey Nick, since .NET 9 came out, there have been some issues with nuget regarding transitive packages. Have you considered making a video about it, or just including it in your next video about .NET 9 (if any), including possible fixes? My brain isn't braining anymore cause of the compile-time warnings about the System.Private.Uri 4.3.0 package having some vulnerabilities. At least for me, it happens only on self-contained publish. Why is it like that?
Ternaries do not lend them self to extension as easily as if statements. I am not generally against using ternaries when there is a clear choice, often in the final return statement of a function, but if the use of a ternary results in a later change just adding into said ternary you easily get something much less readable ;) And having worked a lot with junior developers, they tend to take your example so if you use ternaries, they will, and they might not have the understanding on when they should :D And since junior devs are more likely to not stay, I do not want to end up having to maintain lots of bad ternaries ;)
In my early days I was told that you should only ever have one return at the end of a function. Not saying that I agree, but that's a methodology that is apparently clearer. Personally I prefer the return early strategy as discussed here.
I am confused as to what the benefits are to returning late always. That basically guarantees that you'll need to read the whole function. Could you elaborate on how that is clearer? I might be missing something.
@@Xastor994 In C and similar generation there is no exception handling, so all errors had to be cared via return value. C also had its weird propeties like it automatically assumed that you return the default value, unless you specify. Also, what most important, if you allocated any memory in the function, you had to remember to free it before return (no using, finally or destructors). So before every return, you need to check if the operations were successful, log errors if any, free resources and return the error state. One of the easiest mistakes was to forget doing one of those steps Functions had at least several screens of length (as screens were much smaller), so if you had some early return along the lines it was very easy to miss one of those 'return rituals' in some of the cases It was much cleaner to put all of them in the end of the function as whenever you added some resource allocation, you just added freeing in the end. One of solutions was to add some label before function ending and if you want to return early, you jump there with goto
I'm not used to patterns so I would use data == null || data.Count == 0 I also wouldn't use curly brackets for return early checks, but I know other people don't like them left out. What's worse is the potential int overflow 😮 That alone should disqualify the LinkedIn solution. When you are actually writing functional code there is often a try/catch statement involved. Try condensing that to a single line...
That's not a flaw per se, it could very well be that an exception is appropriate if results that large are generated. try/catch sucks, but `Try` monads exist and facilitate 1-liners if that's your thing
@@adambickford8720 It wouldn't throw an exception though, would it? It would just overflow and wrap around to a negative number (int.MaxValue * 2 == -2). You need to use checked { int.MaxValue *2 } to get an exception and then you need a way to handle that. Or you need to think about all the edge cases you could have and check for them individually. In this case: Is the number bigger than int.MaxValue/2 or less than int.MinValue/2 and what happens if they are. I get that this is an example and chosen to be as simple as possible, but that's the point. Even this simple example has some hidden pitfalls that you need to look out for. Real world applications need to have error handling, no matter if it's try/catch, exhaustive checks or whatever. What happens if the database is locked? Or the API not available?
I'm not against returning null. If return an empty array, how would you differentiate between an empty result and an error? Although, I'd prefer to return a union of in such cases. Otherwise I fully agree with you.
this post hit my LinkedIn timeline as well, if I remember correctly it was posted by a "Microsoft MVP", it's a shame being a MVP means spamming AI generated articles with really bad advice on social media nowadays
The only thing I disagree is "don't use else, ever". If that was the case, then any if-else would have to be its own method and honestly I can't be bothered. Also, instead of ternary operator I'd use null conditional and null coalescence operators.
I think the OP he new exactly what he was saying, the pieces of advice are the right ones. Just the suggested "better version" was wrong, because he probably pasted the wrong thing.
"Never" return empty colloections? I get your point and I agree as a best practice. But if I dig hard enough, I'm sure I could come up with a scenario where a function would have a valid semantic reason for returning either null or empty.
@@Tsunami14 when talking to web apis, 404 and empty lists are two different results that could be expressed in the client code as null or [], although the difference doesn't really matter for further processing in most cases
The way he used the ternary operator is spooky to me. If you'd do this, then at least format it in a readable way: condition ? true : condition_2 ? true : false; With that formatting it's a bit cleaner and easier to follow then this crazy stuff.
I don't get the hate on nested ternary statements. With correct formatting, they are completely readable. If you can't read them, like any language feature, try using them a bit and it'll eventually click. That being said, don't force them unnecessarily.
Just do: data?.Select(d => d*2),ToList() ?? []; And never even create a separate function for something so trivial. (Unless you need to name this piece of code, because it's gotten reused 7 times across the codebase)
It is nullable, but in recent C# versions you can make the compiler treat it as either nullable or non-nullable, similar to value-types. In the end it's just H4XX and has some issues. For example you can't overload F(string s) and F(string? s)
@@theMagos Correct, but even in older versions, just because something CAN be null, you should not always use null, and lists/enumerables and simlar are a clear case where an empty result is so much bette than null since you mostly do not ned extra checks. A simple .Any() or Count() == 0 is much more clear on purpose than "not null" and if the calling code only loops the result, an empty result will just not loop, a null would break with null reference exception unless you add extra lines to check the return. So I rarely use null now and when I do its only when it really makes perfect sense to do so so that anyone using the method more or less expects it to sometimes return null. And top be honest, the more I think that way, the less cases I find where null really does make sense :)
Funniest part is that the longer you read the code, the worst it gets. That is supposedly C# code, however the very first condition uses '≠' character, that is not a valid C# operator. Yes, it is understandable, but still, that won't compile at all.
@@tamirlyn Oh, I did not know this was even a thing. Not something I would use personally, but I can see the appeal. It now makes sense how that piece of code works. Thanks for the reference.
I had a book with code examples like about OOP vs Functional Programming. The examples was so bad, and for some reason someone printed that book. It might be trolling, but it also might not be.
I think he's conflating early return and short circuiting, among other issues. And you've just sent up a flare for every 'well ackshully' tryhard around `null` collections.
Thank you about the advice but there is bad side of this way which is the dominant unfortunately in this career, which is attacking others work without mercy, judging without any consideration to others feeling, we are not machines , give your review in sympathy, thanks the others first about their work and effort then say what you think can fit better , at the end there is many aspects , clean code is just one ,we don't know the circumstances about the code writer
perfect case for a switch expression: public List ProcessData(List data) => data switch { null => null, { Count: 0 } => new List(), _ => data.Select(d => d * 2).ToList() };
I have to disagree. As Nick illustrated, your top two arguments can be combined in to one and the return can be condensed into a simple linq select. The is much less readable AND it also returns a null which I agree with Nick should not be done for List. I go a step further and ensure I always return something.
Sure, early return if it makes sense, but... his "improved" code had no early returns so what's the point? And it was hell to read much less change and maintain. As for the "do not return null collections", sorry, bad advice too, and I'll die on that semantic hill. Empty means no results, null mean "something went wrong". Either a) you are returning something like Tuple and then Empty makes sense because you should check error, or you return null on error. Empty means empty, and gives you no information whether it was really empty or something went wrong. This tripe comes from bad coders not checking their returns, which obviously falls on "check before use", and you're just making lazy people lazier.
Use code BLACKFRIDAY24 at checkout to get 40% off ANY course, bundle or Dometrain Pro: bit.ly/3UVRaWu
The worst about this code is not using the data.Count to initialize the capacity.
Currently the List initializes with a capacity of 4, doubling each time it requires more space.
So lets assume the input parameter has 300 entries:
4,8,16,32,64,128,256,512 -> 8 arrays.
Unnecessary allocation, unnecessary copies, unnecessary wasted memory.
You can write the original code without any ifs or conditional operators
return data?.Select(d => d * 2).ToList()
for the code that returns an empty list when null just slap a "?? []" on the end
Definitely the simplest solution.
Thats the most readable for me.
If the code was longer, I would probably have used something other than just d in the select.
Was about to comment this exactly. The check on Count becomes redundant if you are doing Select().ToList() after all.
? and ?? are conditional operators.
don't need return keyword if you use =>
I have a coworker who this week turned 7 if-else statements into a giant porridge of ternary operators. They must have been reading the same post.
I agree with so many of your points, such as don't return null, return empty. Return early, Avoid else, elegant use of LINQ. I also like the way you stepped through reducing the code, I think so many people get themselves in trouble when they don't do it iteratively.
Golden Rule: accept the _minimum_ you require, return the _maximum_ you produce. If the only thing you do in your method is to enumerate through a collection, then the type of the parameter should be IEnumerable.
IList or ICollection IMO. I used to always default to IEnumerable as well but you don't necessarily know what the implementation is and it could be not safe or efficient to enumerate it multiple times and you wouldn't know if you're the only method enumerating it
@@dennycrane2938 Hmm... What do you mean you don't know what the implementation is? You ARE the author of the method!
It does not matter if you need to enumerate multiple times. As I stated, if the _only_ thing you do is enumerating through the collection, then IEnumerable is enough - that makes the method the most versatile.
You're aware of polymorphism, right?
+1 for inverting ifs, more people need to get on that bandwagon. It's cleaner, easier to read, reduces nasty nesting, etc. The only thing I do differently is I do put EF extension methods on new lines, I feel it makes it better for developers to read the steps that linq is going through against data.
I sometime puts linq on own rows but it depends, simple .ToList or similar I usually leave at the end since they rarely are the important part of the line.
And if I only have one main linq that IS the meat of the row and not to long I also usually do not split the line, split lines also add some extra work for the brain to join together.
But for any longer statement or when I have multiple important linq I also tend to have each on a new line stating with the . to also highlight that its a continuation and not a new statement.
Similar to how I do with ternary when I do use them, I put the ? and : as new rows since that clearly indicate that the line is an extension of the former row and seeing one line with ? and the next with :, at least for me very much screams ternary.
But I rarely use ternary now a day unless in simple cases where the result is readable enough that saving the extra lines is worth it. Its always "which one reads the easiest".
If you use select, you don't need to check if the list is empty either. Agree on everything else though!
but the enumeration of the list is unnecessary
If you use the loop, checking the Count is unnecessary as well (except maybe for slight performance difference)
By simple stretching the code from 3 lines to 4 lines the linked in author could have made the neste ternary operator version so much more readable
public List ProcessData(List data) =>
data == null ? null
: data.Count == 0 ? new List()
: data.Select(d => d * 2).ToList();
This way, it is always condition on the left, "early return" on the right.
That's a legit shocker. Someone who has fallen for the "fewer lines always = better" lie. The compiler doesn't care ... but the next developer who has to read and try to understand the code sure does - you're making it nice for THEM. And THEM might be YOU, 5 years from now, when you'll appreciate clarity.
It's like telling someone travelling light is good, and they take it to mean travelling with nothing is best.
How about this:
List ProcessData(List? data) => data?.Select(d => d * 2).ToList() ?? [];
What do you think?
I understood that line in like 5 seconds! That is an excellent alternative!
The problem screamed elvis operator to me, but it's also a silly toy example where a null input returns null, so your code is wrong. I would say the design of returning null instead of an empty list is a design problem but that's the toy example as it exists.
List? ProcessData(List? data) => data?.Select(d => d * 2)?.ToList();
@@AldoInza Your use of the extra`?` leads to more boxing/unboxing, I feel the code @DimitriKandiner submitted is more efficient and arguably more readable.
@@jasonpirok but it can never return null. I would probably say fix the consumer so it doesn't need a null return, but maybe it's some odd db column or a config parser where null means inherit value etc.
Only thing I usually do different from your solution, is I one-line the if-return (when it's a simple if-statement).
Another option is to use the conditional operator but on multiple lines. That also reads very nicely.
Nullable reference types do NOT prevent you from passing null at runtime - it's compiler air-ware. So there is nothing wrong with null checks in the outer perimeter / public surface.
Dev: Should i do if-else or early-return or ternary-ternary-ternary? Ternary is better right? Fewer lines, faster code?
Compiler: I don't give a shit.
JIT Compiler: I may even make it the same, lol.
CPU: I may change the order of operations, lol.
Dev#2: WTF? Who wrote that shit?
Ternary operators are not the easiest to read when composed. Never be afraid to add a few more lines of code, if that makes the code easier to read.
I often see complicated one-liners with ternary operators from the 'sophisticated' developer, proud that he resolved the problem in one line of code.
Would much rather look at 5-10 lines of code that are obvious and easy to understand in an instant, than having to waste a minute of time, mental energy following the sophisticated one-liner logic.
That's one difference between experienced vs intermediate/junior developers.
So true.
If you really want to reduce the size of a function, break parts out into methods with good names so that you in most cases would not even need to see the inner code to understand what it does
The problem is that someone thought that removing the thumbs down was a good idea
that example most bad reason is not title said,
when process sequence input or output,
1. null is no means (1-has element, 2-empty, or 3-throw when error , can explain all case about sequence)
2. no need to check element exist before iterator (because no element will run 0 times)
then that will be follow
=> data.Select(x=> x * 2);
i may not use fast return to explain it
The Select() call introduces a whole slew of allocations you don't need, like rebuilding a list from scratch without pre-setting a capacity. Instead you should use ConvertAll(d => d * 2).
7-level ternary expressions is the OG way of development
Ternary operators are awful for readability, especially in C, let alone C#. I like your solution as it's better for readability and maintenance. I always validate the inputs and return early as it makes debugging a lot easier.
I'm entirely on your site here. I frequently state that shorter code doesn't necessarily result in better readability.
I think this would be a reasonable way of doing it:
public List ProcessData(List data) =>
data is null ? null // Only returning null to be compatible with OP's example, I agree pattern matching would be better.
: data.Count is 0 ? []
: data.Select(x => x * 2).ToList();
Tho I agree if statements can be more readable.
Personally, I would also not make the method accept a List and return a new List.
Generally speaking, if I take a mutable collection, then I would mutate it rater than returning a new one.
If I don't want to mutate the collection, then I would take a read only abstraction instead.
Ha, they got me.... a code cop about how you should always return early, being a bad thing... I was just about ready to explode.
Yeah their code for what 'you should do' is hot garbage.
When will people learn that LoC isn't at all better for readability, performance, ... nor is it 'a liability for the company', which I hear all the time... CRAPY CODE hurts all of those things... good code is actually readable, performant and actually isn't a liability.... and sometimes it takes 5x the amount of lines to do it, and that's 100% okay.
I feel that code should always tell a story and should be just as reasonable. If I look at a function and it is either 50 lines long or is completely unreadable it goes on the backlog as tech debt or I get permission to refactor it immediately. The primary reason for this is that in either situation, the single use principal is violated which indicates the function was not written with intent. I have told devs for years that the `if else` statement that has a return in the else needs to be redone because nesting is bad and that is the primary cause.
Im 100% with you on returning empty collections if immutable. But I've always wondered about excessive allocation when returning empty for mutable collections, especially if an empty result will be unused.
There is no allocation. Array.Empty or Enumerable.Empty won't allocate.
@nickchapsas Yes of course for immutable types. I was referring to mutable collections like List
@@nickchapsas But List is mutable and will allocate.
Personally I'm okay returning List? For private code to avoid the allocation.
I probably wouldn't do it in public code. But then I rarely return new Lists in public code anyway.
But I don't think sometimes returning an empty list and sometimes returning null (like the example) makes sense in any scenario.
I prefer also introduce a `result` variable before returning, so the return and operation would not be on the same line.
You can also refactor the parameter and make it not nullable and then its just data.select(x=>x * 2).toList() and thats it :D
even with nullable parameter, it's just a questionmark away;
data?.Select(x => x * 2).ToList()
Or if you don't want null return;
[.. data?.Select(x => x * 2) ?? []]
That's not strictly a refactoring, it's a redesign, since you're changing the public api and might break consumers.
A parameter can still be null, even when declared non-nullable, since it might be consumed by something that's not nullable-aware or some silly programmer passes null! . You should always handle the possibility that a parameter could be null in public/protected code.
@@RobinHood70 a programmer not respecting nullability will experience a crash that they need to fix on their side, input sanitation is not necessary on every level in the code
@@Flimzes I 100% disagree with that unless the code is private/internal. As I said, though, it's not just a question of a programmer respecting nullability...not everything is nullability-aware. When they introduced the nullability options, they explicitly said "don't stop checking for null". As a practical example, look at Microsoft's code. It all checks for null in all public-facing methods, even when something's declared non-nullable.
Nested ternary operators are an instant reject at code review. Every. Single. Time.
There's no need now with switch expressions, so it's even more rejectable
@@jessecalato4677 would be nice if EF Core supported it 😢
I've had situations where switch is not applicable and I'd have to chain if-else if-else based on multiple variables. IMO they can be readable if formatted properly (even nested) but that's really a scenario for a kind of switch expression we don't have yet in c#.
@@Xastor994 my instinct would be to do that in nested if/else in a separate method, and not in nested ternary.
@@Xastor994 do you have an example of nested ternary doing something switch expressions can't?
i Quote replied to this linkedin post and propsed pretty much same solution as yours and same points i raised about this bad advise.
Hey Nick, since .NET 9 came out, there have been some issues with nuget regarding transitive packages. Have you considered making a video about it, or just including it in your next video about .NET 9 (if any), including possible fixes? My brain isn't braining anymore cause of the compile-time warnings about the System.Private.Uri 4.3.0 package having some vulnerabilities. At least for me, it happens only on self-contained publish. Why is it like that?
I do not understand the pushback on the unnested ternary expression... cannotReadThis ? 🤦♂️ : 👍
Ternaries do not lend them self to extension as easily as if statements.
I am not generally against using ternaries when there is a clear choice, often in the final return statement of a function, but if the use of a ternary results in a later change just adding into said ternary you easily get something much less readable ;)
And having worked a lot with junior developers, they tend to take your example so if you use ternaries, they will, and they might not have the understanding on when they should :D
And since junior devs are more likely to not stay, I do not want to end up having to maintain lots of bad ternaries ;)
Don't even need to check the count. Select.ToList will return an empty collection if the their input has no elements.
In my early days I was told that you should only ever have one return at the end of a function. Not saying that I agree, but that's a methodology that is apparently clearer. Personally I prefer the return early strategy as discussed here.
I am confused as to what the benefits are to returning late always. That basically guarantees that you'll need to read the whole function. Could you elaborate on how that is clearer? I might be missing something.
one return per function is very old practice
@@Xastor994 In C and similar generation there is no exception handling, so all errors had to be cared via return value. C also had its weird propeties like it automatically assumed that you return the default value, unless you specify. Also, what most important, if you allocated any memory in the function, you had to remember to free it before return (no using, finally or destructors). So before every return, you need to check if the operations were successful, log errors if any, free resources and return the error state. One of the easiest mistakes was to forget doing one of those steps
Functions had at least several screens of length (as screens were much smaller), so if you had some early return along the lines it was very easy to miss one of those 'return rituals' in some of the cases
It was much cleaner to put all of them in the end of the function as whenever you added some resource allocation, you just added freeing in the end. One of solutions was to add some label before function ending and if you want to return early, you jump there with goto
With nullability and method delegates it can be shortened to just:
=>
data?.Select(d => d * 2)?.ToList() ?? new List();
There is no need to be afraid of null, you just need to learn how to cook it.
Lol saw the post on LinkedIn and thought ugh I rather go with first approach 😂
I'm not used to patterns so I would use data == null || data.Count == 0
I also wouldn't use curly brackets for return early checks, but I know other people don't like them left out.
What's worse is the potential int overflow 😮
That alone should disqualify the LinkedIn solution. When you are actually writing functional code there is often a try/catch statement involved. Try condensing that to a single line...
That's not a flaw per se, it could very well be that an exception is appropriate if results that large are generated.
try/catch sucks, but `Try` monads exist and facilitate 1-liners if that's your thing
@@adambickford8720 It wouldn't throw an exception though, would it? It would just overflow and wrap around to a negative number (int.MaxValue * 2 == -2). You need to use checked { int.MaxValue *2 } to get an exception and then you need a way to handle that.
Or you need to think about all the edge cases you could have and check for them individually. In this case: Is the number bigger than int.MaxValue/2 or less than int.MinValue/2 and what happens if they are.
I get that this is an example and chosen to be as simple as possible, but that's the point. Even this simple example has some hidden pitfalls that you need to look out for.
Real world applications need to have error handling, no matter if it's try/catch, exhaustive checks or whatever. What happens if the database is locked? Or the API not available?
Immediately hit the Like button on 'this is not it chief'. Had me in stitches. Oh, and fantastic rest of video too. :p
You know it's bad advice when someone wants to return null from List
1. Where enable???
2. Why null and not empty table?
Long code ❌️
Unreadable code ✅️
For fuck sake man, one just gotta admit the middle point is the best
My first advice is that you are not paid for lines of code, you're paid for working algorithms...
If you where paid by line of code, the given example would be even worse than now :D
@5:45 data.count
That could be a list pattern:
data switch
{
null => null,
[] => [],
List data => data.Select(x => x * 2).ToList(),
}
You don't need the second case, as it returns the same thing as the third when the list is empty.
@louisfrancisco2171 Right. It's not needed in the original demonstrated code either.
I agree with early return if the alternative is pyramid of doom.
That first example... Doesn't even need else because of the returns...
I'm not against returning null. If return an empty array, how would you differentiate between an empty result and an error? Although, I'd prefer to return a union of in such cases. Otherwise I fully agree with you.
this post hit my LinkedIn timeline as well, if I remember correctly it was posted by a "Microsoft MVP", it's a shame being a MVP means spamming AI generated articles with really bad advice on social media nowadays
The only thing I disagree is "don't use else, ever". If that was the case, then any if-else would have to be its own method and honestly I can't be bothered.
Also, instead of ternary operator I'd use null conditional and null coalescence operators.
I think the OP he new exactly what he was saying, the pieces of advice are the right ones. Just the suggested "better version" was wrong, because he probably pasted the wrong thing.
"Never" return empty colloections? I get your point and I agree as a best practice. But if I dig hard enough, I'm sure I could come up with a scenario where a function would have a valid semantic reason for returning either null or empty.
Maybe birthdate, but thar is not a list or collections.
Also I believe you cannot use empty on non collection type
@@Tsunami14 when talking to web apis, 404 and empty lists are two different results that could be expressed in the client code as null or [], although the difference doesn't really matter for further processing in most cases
what is the point of checking for List.Count? If it is empty you get the empty list regardless
Isn't it just: return data?.Select(x => x * 2).ToList();
@@vonn9737 return data?.Select(x => x * 2).ToList() ?? [];
because you want a new List instead of a pointer to the List?
Micro-optimization, you don't create an iterator for Select.
I brought this piece of cake to Nick 😅
Legend. Keep going :D
@@nickchapsas thanks
If I would have had any water in my mouth, the laptop would surely be wet after watching that reveal.
I use an extension method called IsNotNullAndAny which I think reads better
The way he used the ternary operator is spooky to me.
If you'd do this, then at least format it in a readable way:
condition
? true
: condition_2
? true
: false;
With that formatting it's a bit cleaner and easier to follow then this crazy stuff.
I saw this the other day and almost thought about tagging you 😂
I'd just remove the pattern, leaving the || operator for readability, and call it a success.
I wish you had shared your thoughts about returns in IEnumerable functions. There is no way to return early if we use yield. ^^
I cringe every time I open Linkedin. There are so many people writing about things they have no idea of, just to sound smart.
Are you sure this is not a troll post? This must be a troll post, this must be!!!
I wish ;) But its not even close to the worst I have seen, even by experienced devs that are supposed to be good :/
Nope. There are people autistic enough to not see the problems with that approach.
Ha! Thanks for the chuckle!
You're gonna laugh, but i worked with a guy that preferred the first solution above everything else.
i dont like nullable reference simply because it's a compile time check, not a runtime check. the code need to handle null cases
Where dd you find this tip? You didn't provide us the source link where that "tip" is
I don't get the hate on nested ternary statements. With correct formatting, they are completely readable. If you can't read them, like any language feature, try using them a bit and it'll eventually click. That being said, don't force them unnecessarily.
we need witch hunt practices back :D
Just do: data?.Select(d => d*2),ToList() ?? []; And never even create a separate function for something so trivial. (Unless you need to name this piece of code, because it's gotten reused 7 times across the codebase)
Make your code as humanly-readable as possible. You never know when you may need to revisit your own mess.
The compiler should optimize it for you.
I would use a switch expression
wait since when is List not nullable? Isn't it just an object?
It is nullable, but in recent C# versions you can make the compiler treat it as either nullable or non-nullable, similar to value-types. In the end it's just H4XX and has some issues. For example you can't overload F(string s) and F(string? s)
@@theMagos Correct, but even in older versions, just because something CAN be null, you should not always use null, and lists/enumerables and simlar are a clear case where an empty result is so much bette than null since you mostly do not ned extra checks.
A simple .Any() or Count() == 0 is much more clear on purpose than "not null" and if the calling code only loops the result, an empty result will just not loop, a null would break with null reference exception unless you add extra lines to check the return.
So I rarely use null now and when I do its only when it really makes perfect sense to do so so that anyone using the method more or less expects it to sometimes return null. And top be honest, the more I think that way, the less cases I find where null really does make sense :)
@@davidmartensson273 Uh... I never said returning null is a good thing in this case...
@@theMagos Sorry, I just expanded my answer to include one of my pet pewes :/ Meant no disrespect
What's the problem with null collections?
Can't see any problems while reading this snippet 😂
I perceived the author of that piece as a novice, or perhaps a junior? Regardless, I really appreciated your approach!
Ternary operators are perfectly readable if formatted such that lines start with ? and : IMO, it reads to me the same way as an if-else block.
When it is good to use ternary operator?
"Wait for Author"
Funniest part is that the longer you read the code, the worst it gets. That is supposedly C# code, however the very first condition uses '≠' character, that is not a valid C# operator. Yes, it is understandable, but still, that won't compile at all.
Grab FiraCode font mate. Thank me later.
@@tamirlyn Oh, I did not know this was even a thing. Not something I would use personally, but I can see the appeal. It now makes sense how that piece of code works. Thanks for the reference.
Looks like someone forgot they were supposed to be writing C# not JSX
Why don't return nullable List? I am not aware of that. Can you or somebody explain that?
Looks like he's trolling😂
I had a book with code examples like about OOP vs Functional Programming. The examples was so bad, and for some reason someone printed that book. It might be trolling, but it also might not be.
No brackets on the "if clause..."
I would write *=> data?.ConvertAll(x => x*2);*
No `if`, no ternary operators, no `null` in code, no Linq.
I think he's conflating early return and short circuiting, among other issues.
And you've just sent up a flare for every 'well ackshully' tryhard around `null` collections.
Good luck putting a breakpoint in their refactored code... lol
so, the parameter is empty but instead of returning the parameter, you instantiate a new empty list 🤦♂
How about using one character per method? Total optimization and readabilty
We used to play a game similar to "Name that tune" with MUMPs - I can write that code in n characters. 😁🤣
Thank you about the advice but there is bad side of this way which is the dominant unfortunately in this career, which is attacking others work without mercy, judging without any consideration to others feeling, we are not machines , give your review in sympathy, thanks the others first about their work and effort then say what you think can fit better , at the end there is many aspects , clean code is just one ,we don't know the circumstances about the code writer
I thought next video will be about .Net 9 launch
Everyone is doing that. I want some time for the dust to settle to make a good video, not a hasty one
I break ternary before :? characters, so it would look like:
```
return data is not { Count: > 0 }
? []
: data.Select(d => d * 2).ToList();
```
What do you think about this:
IEnumerable ProcessData(IEnumerable? data)
=> data?.Select(d => d * 2) ?? [];
It seems as though someone has decided to troll Nick with intentionally ridiculous advice.
Fun fact, someone has sent me their own advice for a code cop video. Didn't do it ofc
Just use switch expression
Some people need to stay away from coding all together…
The person under this post must be trolling there is no way 😭
I kind of want to start creating bad advice just to see if it bubbles up to Nick...
perfect case for a switch expression:
public List ProcessData(List data) => data switch
{
null => null,
{ Count: 0 } => new List(),
_ => data.Select(d => d * 2).ToList()
};
I have to disagree. As Nick illustrated, your top two arguments can be combined in to one and the return can be condensed into a simple linq select. The is much less readable AND it also returns a null which I agree with Nick should not be done for List. I go a step further and ensure I always return something.
what about [data?.Select(d=> d * 2)]
You'd get a compile error trying to return that
@@theMagos oh yes sorry my bad. Than you have to cast and the code is ugly again.
This code looks like someone used ctrl + . too often in VS
Sure, early return if it makes sense, but... his "improved" code had no early returns so what's the point? And it was hell to read much less change and maintain.
As for the "do not return null collections", sorry, bad advice too, and I'll die on that semantic hill. Empty means no results, null mean "something went wrong". Either a) you are returning something like Tuple and then Empty makes sense because you should check error, or you return null on error. Empty means empty, and gives you no information whether it was really empty or something went wrong.
This tripe comes from bad coders not checking their returns, which obviously falls on "check before use", and you're just making lazy people lazier.
1:10 not readable nor future changes friendly