"One Line of Code Means Clean Code!" - Code Cop

  Рет қаралды 56,134

Nick Chapsas

Nick Chapsas

Күн бұрын

Use code CLEAN20 and get 20% off the brand new Clean Architecture course on Dometrain: dometrain.com/...
Become a Patreon and get source code access: / nickchapsas
Hello, everybody, I'm Nick, and in this video of Code Cop (formally LAPD) I will take a look at a LinkedIn post that is using Clean Code and KISS or Keep It Simple Stupid, to provide so really bad advice on clean code.
Workshops: bit.ly/nickwor...
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: bit.ly/ChapsasG...
Follow me on Twitter: bit.ly/ChapsasT...
Connect on LinkedIn: bit.ly/ChapsasL...
Keep coding merch: keepcoding.shop
#csharp #dotnet

Пікірлер: 459
@DjDanny32
@DjDanny32 Жыл бұрын
Debugging is harder on the single-line version. Any exceptions get thrown on the single line, and you won't easily know which part of it is causing the error.
@vothaison
@vothaison Жыл бұрын
Yep. Sometimes I have to break the line to debug and then put the lines back together for so that the code reviewer doesn't complain.
@xparadoxical69
@xparadoxical69 Жыл бұрын
that's a debugger/ide issue, not a code issue
@Cristian-ek7xy
@Cristian-ek7xy Жыл бұрын
I agree a 100% to this. We do not need to compact/compress the code, code is maintained by humans, not machines.
@benjamininkorea7016
@benjamininkorea7016 Жыл бұрын
@@IIARROWS When I have a mysterious error, I step through the code line by line until I hit it. Assuming, "Well, this is simple it will never go wrong" might be correct in this case and the next 20. But the 21st is gonna screw you.
@williamforsyth6667
@williamforsyth6667 Жыл бұрын
"Debugging is harder" Only soy developers debug. (at least according to similar videos)
@weihog
@weihog Жыл бұрын
In my opinion all of them, except the Fast one, are equally easy to read. For longer variants I would prefer the "Bad" example. But the most importent point to me is debugging. It's much easier to set breakpoints, look at varaibles and find buggs in the "Bad" example.
@adambickford8720
@adambickford8720 Жыл бұрын
Is this a visual studio debugger limitation? With intellij i can just mouse hover or alt+click to see the result of any value or expression. Worst case you can set arbitrary watch points and use conditional breaks.
@the_lenny1
@the_lenny1 Жыл бұрын
​@@adambickford8720nope, it's the same for vs. you can also get all values with mouse over, but you of course can't set a breakpoint inside a line but only one a single line. or you go into the jitted code and set a breakpoint there on the exact instruction you want xD
@adambickford8720
@adambickford8720 Жыл бұрын
@@the_lenny1You can really only set 1 breakpoint per line? How do you stop on the execution of a lambda vs the definition of it for a 1 liner?
@emerynoel567
@emerynoel567 Жыл бұрын
@@adambickford8720 We can set breakpoints on lambdas in VS. There's a lot of flexibility, but not unlimited flexibility. We can also hover, or highlight multiple expressions and Alt+F9, to see the value. If you're just chaining a bunch of methods, I don't think you can breakpoint on an individual link in that chain (unless it's in the internal of a lambada).
@grazy27
@grazy27 Жыл бұрын
Also easier to see line coverage while writing unit tests
@DoeJohn333
@DoeJohn333 Жыл бұрын
As a junior dev who has a lead programmer who prefers code broken down and also has a senior dev who is always telling me “ this can be put into a single line” at every PR review… being a junior dev is tough when everyone isn’t on the same page
@Zalmakiz
@Zalmakiz Жыл бұрын
this was very annoying to me to, never could ever explain me why things are better when they are in one line.
@NWessel
@NWessel Жыл бұрын
You are completely right, that is a very hard place to be. What is important for you, is not to choose the right thing, but rather get the understanding of the pros and cons of both ways! Keep your own focus on the learning as much as possible. Not always possible.. I know :) Just evolve yourself - From some senior dev
@johnnyblue4799
@johnnyblue4799 Жыл бұрын
Get them together and tell them exactly what you said here. I'd side with the one who prefers the broken down code though. I write one liners many times myself, only to break them later while debugging stuff. While the one liners look 'cool' I find them harder to work with.
@DevMeloy
@DevMeloy Жыл бұрын
As a senior dev, I can understand the frustration and this is a good example of why companies need upfront coding standards. My current employer has almost no standards which causes our code base to be a hodge podge of syntaxes... when I was a Jr. dev, my mentor told me he shouldn't be able to tell who wrote the code by looking at it. Meaning, conform to whatever standard you're currently working in and you should be fine.
@tmhchacham
@tmhchacham Жыл бұрын
@@Zalmakiz One-liners are better (to me) because once you know what they do, it's very simple when reviewing the code. When there's a bunch of line, it'd really confusing (and ugly!) Furthermore, the one line shows it's all "one" action. But to each their own.
@billy65bob
@billy65bob Жыл бұрын
5:00 If you want my opinion... I do like the 2nd one better, but only marginally, and that's mostly because I prefer short snappy functions, and I like method chains. I dislike the 'new string' bit though, mostly because it makes the logic progress middle-right-left, whereas the 'bad' one goes top-middle-bottom in perfect sequence. EDIT: And that's an issue both your revised approaches avoid, lol.
@Pugsly4000
@Pugsly4000 Жыл бұрын
yeah i agree, the only thing that threw me a bit was seeing the "new string", which i haven't seen often (or at all), but otherwise i found the "good" one more readable, it reminds me of Fluent Assertions for example which ironically Nick uses all the time in his UnitTests :D
@HMan2828
@HMan2828 Жыл бұрын
As a lead dev who has to cuddle and teach 3-4 junior devs, working on ERP systems, 99% of the time I don't care about performance or memory usage, unless it causes obvious problems. I don't chase nanoseconds or allocated bytes. I get that in some cases it's vital, but in our context it doesn't matter much, like I said unless there is an obvious performance problem. Instead, I focus on teaching them to write well structured self-documenting code, with proper indentation, good naming conventions, etc.. The performance optimization part comes in at peer review if necessary.
@piotrszuflicki1527
@piotrszuflicki1527 Жыл бұрын
Actually you lose most of the time on db calls generated by ORM, that for some reason are insane. That's one of the most crucial skills Junior developer should learn.
@logantcooper6
@logantcooper6 Жыл бұрын
​​@@piotrszuflicki1527well written, basic CRUD queries with a decent ORM doesn't sacrifice that much performance. Some people just abuse them and give them a bad name. Or don't know when to drop down into SQL. It's a skill issue.
@HMan2828
@HMan2828 Жыл бұрын
@@logantcooper6 Agree 100%. Most of these bad queries would still be bad queries if it was written in SQL. Common thing I see is writing a DB query and applying a cast on the DB field in a join or where clause. Junior programmer has not figured out how to conceptualize server vs client context yet, and doesn't realize his code forces a scan on ALL the records in the table... An ORM is a tool, and like any tool there is a good way to use it and multiple bad ones.
@davidmartensson273
@davidmartensson273 Жыл бұрын
@@piotrszuflicki1527 True, as soon as any DB calls or other external calls are involved, those are more or less the only thing you need to optimize performance vice, nothing else will even be noticeable.
@StarfoxHUN
@StarfoxHUN Жыл бұрын
I just realized something. The actual 'clean' way to turn this into a one-liner is just... Using the method created here. When you create this method, you create the "One liner" you will use later on in your code. In this case, its not about "how much line do i need to reverse a string", but "okay, i have a method that reverse a string, now i just simply have to call that in one line/one simple call whenever i need it"
@evancombs5159
@evancombs5159 Жыл бұрын
Yup, and I think ideally you would do this as an extension method.
@carlosalbertohernandezmogu7684
@carlosalbertohernandezmogu7684 Жыл бұрын
In this specific example, I do actually feel the second option is "cleaner", it allows you to read the rest of the code in the screen without having to scroll, one liners may help you understand the context of the rest of the code allowing you to see more in a single look. But I totally agree, there should be a limit to the size of those "one-liners" in order to keep your code readable for others.
@TheCameltotem
@TheCameltotem Жыл бұрын
What is this damn obsession with having things as short as possible? Just because it takes up less real estate doesn't make it easier to understand or read. It's like you start removing letters from a sentance so you can write and read quicker. No one does that with the English language so why should we do it with the C# language. Now this is a trivial example but nonetheless. The first example will be easier to walk through, especially if you're a junior. It's also easier to debug if something were to go wrong, instead of having to understand what went wrong in the line that pretty much does 3 things already.
@andiwand1119
@andiwand1119 Жыл бұрын
In this specific case I would say I prefer an approach in which I can provide more debugging steps. The oneliner is harder to debug (or I maybe I just dont know how to do it properly), but the "bad" solution offers a better way to check the "interim results". You version is the best for "readability" I would say.
@SkandiaAUS
@SkandiaAUS Жыл бұрын
Sometimes when you aggressively inline code it makes it really damn hard to debug in production. A good example (don't ask how I know) is when you try to be smart with LINQ and bundle some huge ass query into a one liner. In production if any of that throws you get the line number, but not much of a clue what part of the query failed (depending on the error). I often explode the code to be easier to identify in production. I have a fair amount of Rider "suppress this warning" statements in my code when it tries to suggest minification.
@rGunti
@rGunti Жыл бұрын
I often like to give Java a bad rep for _basically anything_ but one thing that Java does well with debugging is line-by-line debugging. When you have a method chain, like you'll get with LINQ, the Java debugger is often smart enough to let me step through each of the chained method calls one-by-one, whereas in .NET, if I chain together 10 method calls to one, it will just run this as one block and if it fails, better figure out yourself how that works. Then again, with .NET I can move the debugger pointer around and rerun sections of code multiple times, which I always miss in basically any other language.
@davidmartensson273
@davidmartensson273 Жыл бұрын
This one I also have to much of the wrong experience with ;) Long linq statements are very hard to debug, and as shown, most likely not faster anyway.
@evancombs5159
@evancombs5159 Жыл бұрын
I inherited some code like this. They first time we had a need to change the code I refactored the whole thing into different methods to represent each step of the process. At a high level it is still a single LINQ query, but the details are now separated into self contained methods so you can actually debug issues.
@Nerdsown
@Nerdsown Жыл бұрын
I frequently create long linq chains because it is easy to read once you know the functions. Algorithms start to take very specific forms that are easy for me to pattern match against. Because each extension does a specific thing its hard to abuse or get wrong. The dev is forced to adhere to the single responsibility principle instead of having giant for loops that solve multiple problems co-currently, and can't be easily decoupled. When you "explode" the code, you're making it so much harder to understand because everything becomes a for loop that does a million things. I very rarely get exceptions from linq extensions unless they're "enumeration stepped while changing" or null ref, which are usually trivial mistakes. So I am very curious what the exceptions were that were hard to debug. Nesting linqs beyond 2 levels however is bad and where it becomes unreadable. But a single level can be 100 chains long and its gorgeous.
@evancombs5159
@evancombs5159 Жыл бұрын
@@Nerdsown I would guess it is that nesting that he is complaining about. That was the issue I was speaking to in my example. If there is no nesting, even if long, it is usually easy to understand. I would wrap a long LINQ chain in a method to keep the calling method cleaner.
@L1da77
@L1da77 Жыл бұрын
Keep it simple is key here. Doing everything in a single line doesn't make it easier. Less lines of code but way harder to read. The example is very easy overall but I'd say that in most cases keeping one operation per line is the easiest to read and less prone to bugs.
@tofu1687
@tofu1687 Жыл бұрын
What ?!? There is a solution about string manipulation where Nick doesn't use span ?!?
@dance1211rec
@dance1211rec Жыл бұрын
Don't worry. The fast solution uses span!
@winchester2581
@winchester2581 Жыл бұрын
It doesn't have any sense. Like... we're creating the new instance of string, and we definitely SHOULD NOT manipulate the values of original string while using span. But If this comment is a simple joke, I have no objection :)
@riten
@riten Жыл бұрын
Except that he did use spans, the function takes a span of char, the string is just automatically being cast to a span. :)
@riten
@riten Жыл бұрын
@@winchester2581 The span just servers as a fast reading technique. The modified values are on a different address. So it makes sense to use spans. And it does use spans under the hood.
@scrambles56
@scrambles56 Жыл бұрын
Honestly, I'd just put the fast approach into an extension method, and then keep string reversing logic away from the business logic. When you're working on the extension method, I don't mind if it's not "clean", its a single purpose function that you already know the intent of when you begin looking at it.
@HCRitter92
@HCRitter92 Жыл бұрын
While I respect your perspective, I personally prefer sticking to the 'Good' side too. Reading from left to right and seeing what unfolds next makes it more intuitive for me. It's like the flow in PowerShell, where everything pipes together, even if it involves dotting in this case. Different approaches work for different folks, and that's the beauty of it - we can adapt our methods to what suits us best!
@polysillycon
@polysillycon Жыл бұрын
For some reason, the ML Java / Apache Spark community is obsessed with creating massive one-liners in their code. It’s like some kind of badge of honor to create barely readable scripts to do simple tasks.
@parlor3115
@parlor3115 Жыл бұрын
The LinkedIn one-liner is still more readable than the first approach, though. But, since ReverseString is a common utility function that's going to be written once and used all over the codebase, aesthetics and readability don't really matter and you should focus on performance.
@jiebaef
@jiebaef Жыл бұрын
A friend of mine is obsessed with one-lining everything in python but only does it for shits and giggles (e.g.: he took a ~250 line program I wrote during a hackathon and compressed it down into one line just to demonstrate to me what weird shit you can do in python). He'd never actually commit any of that to their codebase
@Krokoklemmee
@Krokoklemmee Жыл бұрын
@@jiebaef you can write entire programs in a single line in almost every language, it's not really specific to python
@ara849
@ara849 Жыл бұрын
Line through screen, line not worth.
@onman999
@onman999 Жыл бұрын
When I just started, I was able to compress all sorts of extensive calculations and comparisons into a single line of code. Until I found out that after half I year it took me too much time to decipher what exactly it did, so I started breaking it down into comments. Now I'm experienced I just use some more lines just to make things easier readable. The compiler's optimizer will do the rest for me.
@KnightofWine
@KnightofWine Жыл бұрын
I always go for performance. If the ugliest code ever is faster, I'll use it. I work processing Gigas of data every morning, and speed is everything to my clients so they have their data available in time. Of course, nothing is carved in stone here, and you need to check your situation before deciding.
@deredware9442
@deredware9442 Жыл бұрын
For me the readability of both is fine. I somewhat prefer the second just because I don't have to carry context over from other lines to figure out what a single line is doing. I know what all the bits of that one line do, I don't need extra lines to pick them apart. My preference would be the fast version though almost every time. This is a method you write once ever, do it the fast way the first and only time. I think the 'fast' method you showed isn't actually the fastest it can be. Reverse-iterating the input with a for loop and assigning inside it would be faster I think than copyto + reverse, though it would be difficult to read.
@ws_stelzi79
@ws_stelzi79 Жыл бұрын
l mean if you already mushing it all together in ONE return statement you really have to loose the curly braces and do a "=>" construct! Just one line compared to 4 MUST be less code! 😉😇🤪
@yakovkemer5062
@yakovkemer5062 Жыл бұрын
Very clean code. var a = b ? c : d ? e : f ? g : h; Not possible to read, will forget what it is doing in 2 days. Code must be readable, not small.
@gakera
@gakera Жыл бұрын
This video was like 5 mins of discussing meaningless changes to code and then an interesting Fast method with no explanation of why it is faster :(
@Javilingys
@Javilingys Жыл бұрын
I write one-line "better KISS" code because My team lead angry to me if i don't write single line code in such cases and Code Review rejected.
@TazG2000
@TazG2000 Жыл бұрын
The "bad" method is more explicit about the details, but that is not always simpler. I think the real problem here is the fact that it's awkward to build an array and pass it to the string constructor. When doing something awkward or unintuitive, it can help to make the steps more explicit. But the most readable version *would* be a declarative one-liner with well-named methods that fit the purpose. "string.Concat" is an improvement but not ideal for the purpose. I think the cleanest solution would look something like this: static string AsString(this IEnumerable input) => new string(input.ToArray()); string ReverseString(string input) => input.Reverse().AsString(); Now we have a well-named general purpose method, and the usage is obvious. It's not the fastest, but if we need the highest performance then readability isn't the priority anyway - that high performance code can be hidden away in a library and just provide a public interface that is easy to use and read.
@Nworthholf
@Nworthholf Жыл бұрын
Thats by far the best approach readability-wise
@BenMakesGames
@BenMakesGames Жыл бұрын
in terms of readability, the "good", "bad", and "nick" seem close enough to not matter. if I saw either of these in a PR, I don't think I would question either one. (my bigger question would probably be "they needed to reverse a string? what are they doing in here??") if I saw the "fast" code, I'd probably be like "wtf is all this??" unless there was a comment, or other context, making it clear "this is a hot path! do not refactor without performance testing!"
@Ristogod
@Ristogod Жыл бұрын
Yes, the 2nd is much simpler and much easier to read. I can't see why it isn't. But I would prefer to work it so it used fluent-style extension methods.
@christopherwood6514
@christopherwood6514 Жыл бұрын
It actually took me less time to understand the shorter function than the first because in the first one, I had to keep all the variables and context in my head, whereas on the second one i could just easily see that the string is reversed, then copied. Adding the extra lines doesn't help because it is such a simple function that adding lines just obfuscates it a little bit
@ymko2099
@ymko2099 Жыл бұрын
Found it funny that the "bad" approach was much faster
@davidmartensson273
@davidmartensson273 Жыл бұрын
Not surprising, Linq and Enumerable is rarely the fastest since its quite a lot of overhead. It can make for easy to read code (if you stay away from long one liners) but compared to arrays its very much slower.
@blacklotuslv
@blacklotuslv Жыл бұрын
maybe not in this exact example, but in more complex code, mushing everything together in one line is a pain to debug. "Error on line 69" is useless info if all the code is on line 69.
@JeffGraw
@JeffGraw Жыл бұрын
Yeah, I disagree with this take quite strongly, at least for *this* example. I would even go one step further, get rid of the braces and make it into an expression-bodied method (on two lines). The second version has significantly less characters and reads *perfectly* from left-to-right. Reading one line from left-to-right is obviously much easier than reading left-to-right then up-to-down and repeating that multiple times. And if we're talking about a 200 line file vs a 600 line file, where the later expands as much as possible into multiple lines, the former is far easier to navigate and take in at a hundred foot level. Now, I did say "at least for this example." Compacting everything into a single line is not always the best choice. We always need to be pragmatic and avoid inflexible rules because there are always edge cases. At the extreme, one-liners can become like run-on sentences, and it's not always possible to read a line perfectly from left-to-right. You might, for example, have to jump between sets of parentheses to decode the meaning, and that's not good. This example however, is an exemplar for when you, as a pragmatic programmer, should go for the one-line version.
@brianviktor8212
@brianviktor8212 Жыл бұрын
Let's abstract the problem, meaning what would we prefer, a loaded line of code, a well readable multi-line code or an optimized one that's harder to understand. I prefer function over form, so I'd pick the performant one, but only if it is relevant. The other 2 I don't care about - it's a question of execution (how it's written) to tell which is better. If it's something more complex, I prefer to use multiple lines as to make it more understandable. If it's something trivial, I prefer one-liners.
@jongeduard
@jongeduard 11 ай бұрын
First of all, thanks for mentioning the string Create method, I was not aware of that one yet, even though I knew about Spans in different use cases already. Time to dive further in them. When it comes to using multiple dots coding style and making it more readable, my very simple answer is that you can always place these on next lines, especially in combination with LINQ methods and other functional style programming C#. In this case, the methods Reverse and ToArray are really candidates for that. But the thing is, C# is still not fully optimized to deliver the best performance with that style. Even though in a part of the cases LINQ methods can actually be the more efficient choice, in other cases they are not. And strings are indeed really critical, everyone should know that. I would say, the most important thing is to learn understand all these things and to always consider when performance matters the most or when code readability, and maybe simplicity, matters the most. It depends on the situation.
@johnbennett1465
@johnbennett1465 Жыл бұрын
As someone who programs in other languages, all versions are bad. The fact that you need to go through this completely is a language design error. The code should read "input.Reverse()". There should be no reason to even create a subroutine. If there is some fundamental reason for that to not return a string, then use "String(input.Reverse())". Needing anything more is a language flaw.
@baranacikgoz
@baranacikgoz Жыл бұрын
As a fan of you, I recommended span approach in the comments of this post once I saw the awful codes in image. Now I see you you did the same thing in video, I became a span monster day by day with you :)))))
@dcernach
@dcernach Жыл бұрын
The one line code is a reading nightmare, hard to debug and is ugly! I'm much more into readability than shortness! Extreme performance reasons excluded...
@nitrous1001
@nitrous1001 Жыл бұрын
Code Cop sounds like a better name for a series since it sounds like an existing analyzer like Style Cop.
@amuuuinjured
@amuuuinjured Жыл бұрын
I would choose one line ower three - without creating any variables. For me "return string.Concat(input.reverse())" is the inital way to go. There is nothing you can break in such code. You can create new one - Faster, but you cann't break it and there is nothing to debug it is pretty straightforward
@AduMaster
@AduMaster Жыл бұрын
I didn't know that `string.Create` and `string.Concat` exist 😀
@nazar5549
@nazar5549 Жыл бұрын
You can't just reverse an unicode string by reversing chars. You'll get wrong results for emojis and many languages
@duramirez
@duramirez Жыл бұрын
what if you do it binary tho? 👀 🤔 it would be slower I suppose.
@riten
@riten Жыл бұрын
Gotta love strings.
@bluecup25
@bluecup25 Жыл бұрын
@@duramirez What do you mean doing it binary? Reverse it bit by bit?
@QuAzI_NODE
@QuAzI_NODE Жыл бұрын
You are confusing characters and bytes. So in unicode strings with unicode chars you can do it in 99%
@welltypedwitch
@welltypedwitch Жыл бұрын
@@QuAzI_NODE emojis can take up multiple unicode codepoints, even in UTF-32. E.g. 🏳‍⚧ is made up of 🏳, a zero-width joiner and ⚧
@allinvanguard
@allinvanguard Жыл бұрын
You know what makes 0 sense to me? This person is literally writing a method to create a one-liner for any consumer in order to reverse a string. If you are already creating an "easy" facade, why would you make the implementation behind harder to read? You are literally centralizing your logic into a specific place to make it easy to maintain. Why would you make it harder for yourself? ALL consumers of this method will have a one-line API to consume anyways. As long as a detailed, "longer" implementation does not mean the code runs significantly slower, it's just very impractical advice.
@ryan-heath
@ryan-heath Жыл бұрын
I have seen projects where most methods were one liners. They took the clean approach to the extreme. Say goodbye to readability and maintainability 😂
@MaiconLLoti
@MaiconLLoti Жыл бұрын
Moreover, if someone is willing to create an extension/utility/facade, why not focus more on performance than on maintainability? It's assumed that this kind of method will be used throughout the project's scope and is unlikely to be revisited. Of course, he probably used the context there just as an example of the KISS principle, but without a good context, it only serves to confuse and make others think that one line is always better.
@allinvanguard
@allinvanguard Жыл бұрын
​@@MaiconLLoti Indeed, if I make the effort to make a piece of code reusable like this, I make sure it's as fast as it can get. It's a balance of maintainability and speed depending on the context.
@denisthedev
@denisthedev Жыл бұрын
Clean code - nobody can be right ever :D I love clean code, but the fact that is so subjective is just killing me. Honestly, I do not care about any of the Linkedin examples. The second example is for me like "oh new string(input.Reverse() ...) got it". I would not call any of them worse or better. But I do agree, that the first one probably needs less thinking about what is happening. The "one dot per line" sounds interesting, but for me again, it depends. If the methods you chain are called properly, then I do not care that much. Even with multiple dots, you can still put them on a new line (technically still the same line, but whatever) to make it more readable.
@Saleca
@Saleca Жыл бұрын
I like your version but i dont mind one liners if they are readable. I just think it is a crime to use blocks in that case, i personally much prefer expression syntax, making it "string ReverseString(string input) => string.Concat(input.Reverse());"
@Xastor994
@Xastor994 Жыл бұрын
Rather than avoid multiple dots per line, I find the greatest readability improvement in avoiding nested brackets (of any type) in a single line, since in order to read that code you need to go back and forth which is generally more difficult. Extracting variables like this is also a good test of code understanding, since if you struggle to find a good name for an intermediate variable, there may be an issue with the approach.
@exmello
@exmello Жыл бұрын
Exactly, while you can read the code left to right to understand it, it doesn't actually evaluate in that order with that nesting. I don't mind more than one dot per line if it's an extension method for mapping/casting/conversion or using a builder pattern, but once you start evaluating out of order it should be broken up. Honestly, I wouldn't call the single line in this example "bad" as I don't have the luxury of being that nitpicky in a code-review, I just take actual issue with it being called better.
@Xastor994
@Xastor994 Жыл бұрын
@@exmello Yeah exactly. The situation in the example is just making a string out of a character array which is not a huge issue and I would let it slide, but imagine if it was a more complex passing of an argument into a function or constructor.
@lordicemaniac
@lordicemaniac Жыл бұрын
i prefer single line of code as long as its still simple, the one in example was simple to understand i would use it like that
@MayronWoW
@MayronWoW Жыл бұрын
I prefer not using `var` in situations where it's not obvious what the type is, so that `new string(input.Reverse().ToArray())` line is hard for me to code review. I had to go into my editor to figure out what the returned types were to see if it was needed. For a moment, I thought `input.Reverse()` might have returned a string, making everything else pointless, but it returns `IEnumerable`. I much prefer the top version because I know what's happening and the types being used. Also, I prefer more lines of code rather than several statements merged into 1 line for a number of reasons: - I can put breakpoints on separate lines and inspect the values of variables much more easily. - The exception/stacktrace information reported from log error messages is much more exact and therefore much easier to fix bugs because the line number would point to only 1 or a couple of operations instead of several on the line at fault; In the original example, if the stacktrace suggested that the `new string(input.Reverse().ToArray())` line was at fault then was it `input.Reverse()` causing the issue, or `ToArray()`? - Keeping a large number of operations on a single line instead of breaking them up into multiple lines is the equivalent of someone forgetting to use any fullstop in a huge paragraph versus using many to break up the pacing; it's just much easier to read and understand. EDIT: Got to the end of the video and yea, use string.Concat for readability and simplicity, or the fast version for memory optimisation scenarios.
@TheFantasticMrFish
@TheFantasticMrFish Жыл бұрын
A lot of people equate less lines with simplicity but that often leads to these issues. You're free to make the entire program one line if so wish but nobody thinks that's a good idea (JS minifiers excluded!) For the exact same reasons, it's probably not a good idea for your functions either
@7th_CAV_Trooper
@7th_CAV_Trooper Жыл бұрын
Entire program one line... This makes me think of a book with no punctuation, no paragraphs, no whitespace, just a series of letters. :)
@john_paul_r
@john_paul_r Жыл бұрын
This particular one-liner doesn't bother me one bit. Transforming enumerables as a series of chained steps is a very natural way to operate, for me at least. The intermediary variables just don't offer much to readability for me here (its just a level of visual indirection as far as I'm concerned) I think that for more complicated chaining scenarios, it can be useful to have "checkpoints" -- when you assign something to a variable to give it a name. I just think this one is too small to benefit from it (I don't need a name to tell me that the output of `str.Reverse()` is a reversed sequence of chars. In a longer chain, such a named var might be useful, because it makes it easier to follow how the enumerable is transforming (or even to skip the beginning of the chain when skimming))
@AmateurSpecialist
@AmateurSpecialist Жыл бұрын
The "good" example is the programming equivalent of a run-on sentence. It's fine if you're playing code golf, but if other humans need to read it, give yourself some extra lines to make the intent clear.
@0nly0nes
@0nly0nes Жыл бұрын
I think the single-line example is "cleaner". Personally, I read this line like a text and skip the less important parts of it in my head. I read the name of the function and know this should reverse a string. Next I read input.Reverse(), which is all the function does. The rest is overhead, which is needed for the string to build. If you want to have a single dot on each line, you can still add linebreaks, which I like to do in such cases. Furthermore, I want to know what is going on, with a single look. If you have more characters and more lines, you will take more time to read everything and maybe even ask yourself, why something is used there, which is oftentimes not important. Your Nick Example is cool, but now there is a Concat call, which normally is used in a completely different way and I would ask myself, what the reason of this is, since it seems like a little hack. For this example, I would implement an extension method, which is implemented in one of those ways, and from there on only use the extension method. E.g. var s = "Hello"; var r = s.Revert();
@PedroPabloCalvoMorcillo
@PedroPabloCalvoMorcillo Жыл бұрын
Yeah. Exactly my case, too.
@onman999
@onman999 Жыл бұрын
I second this.
@kylekeenan3485
@kylekeenan3485 Жыл бұрын
I think your approach is great if your a clever Dev working with other clever Devs, however in reality a lot of teams have a variety of Devs at different levels of skill and experience. I find Nick's example more appropriate in terms of keeping it short but easily understood by all. That way everyone has an easier time maintaining and developing the applications.
@davidmartensson273
@davidmartensson273 Жыл бұрын
If I would do a one liner I would add an extension method for the char array to create the string, the current one mixes order, it first starts with the new string and then as parameter have chained methods, so the order is not consistent. And sure short one liners can be readable but in my experience, once you start doing one liners its easy to keep doing them and you do not have to go much beyond this length before it gets quite a lot more hard to read. Still, with a good method name that is not a big problem since for one liners, most of the time you do not need to look at how it does things, only what it does. So for this small example, I would probably not object to either one solution except the fast one. Yes its faster, but its also quite a lot more complex to understand so unless your REALLY doing very high performance code I would object to it. Its code that should only ever live hidden away inside some library :P
@leandroteles7857
@leandroteles7857 Жыл бұрын
I personally find the one-line variant very readable. And especially in C#, I like to keep methods in one line when possible, because it allows you to use expression-bodied members, which removes the wasted space from line breaks and brackets, thus allowing you to view more of your code at once. Plus, this is the type of method that I would probably keep as a extension method in some place I don't need to see. That way the only thing I will be seeing every day is "somestring".ReverseString(), which is self-explanatory.
@davidmartensson273
@davidmartensson273 Жыл бұрын
On the other hand, since its so much slower, if you pack it as an extension method you most definitely want the "bad" or "fast" version, since you do not have to look at the code anyway :)
@onman999
@onman999 Жыл бұрын
When coding there are 3 possible approaches: 1. Easiest readable. 2. Most efficient 3. Least amount of code KISS should be the first approach and therefore the 'bad' example should be used: it is the quickest to understand. The 'Fast' example is obviously the most efficient, but has more parameters and is therefore harder to read than the 'Bad' example (and thus less KISS). The 'Good' example is the least amount of code. At the end of the day, most code is a balance between the 3 approaches. Or as we say in the Netherlands: the truth is always in the middle.
@EmptyGlass99
@EmptyGlass99 Жыл бұрын
'Concat' when reversing a string is more confusing to me than the other two examples.
@DemoBytom
@DemoBytom Жыл бұрын
Yeah exactly.. Why would I concat only one string? That implies joining together two or more strings, which is not what the method should be doing in the first place. Readability wise it was a massive regression for me.
@ajar1000
@ajar1000 Жыл бұрын
I like less lines of code because you can fit more on the screen. For the reverse string array, both implementations are fine imo, but I would personally strive for the second option
@StarfoxHUN
@StarfoxHUN Жыл бұрын
For me, the main reason i hate these 'one-liners', is not just it generally much harder to read, 'debugability/debug experience'. When you have a one-liner like that, debugging that is pain, i often had to separate them to individual lines to find what the exact problem is.
@ivanp_personal
@ivanp_personal Жыл бұрын
In general, here I'd vote for the "fast" implementation, even despite it is not best readable, because the speed is what we really want from such utility functions.
@7th_CAV_Trooper
@7th_CAV_Trooper Жыл бұрын
Speed is only one of the things we want. We also want the code to be correct. Math.Add can always return the wrong answer very fast if it doesn't bother to actually add. We also want the code to be maintained by a team of humans who need to be able to comprehend it.
@ivanp_personal
@ivanp_personal Жыл бұрын
@@7th_CAV_Trooper Yes, that's correct. We of course want the code to be correct. But given it is correct, next things we typically want are speed and low memory usage, preferably, zero memory usage.
@rmcgraw7943
@rmcgraw7943 25 күн бұрын
I am very bad about writing 3 line of code that replaces 300 lines. It’s not a good practice, as it’s inpossible to debug or maintain but, for me, it’s an optimization step at the end of my method creation. I generally keep the 300 line version (commented out), for the support team.
@killymxi
@killymxi Жыл бұрын
For readability, I would prefer chained code (broken into lines where necessary) - I just see that it is a single expression, the code flow is simple. Properly named methods in one chain can "tell the story" clearly enough. Absence of throw-away names means the remaining named variables are important. I can give names to intermediate results even if they are used once - when descriptive names are really important there. (But I regret my style choices sometimes when debugging - have to get back and break expressions into statements to step through them...) The example in the video has one thing I can frown upon though - expression doesn't read from left to right - it ends with a constructor that is on the left. In such case I may also break it into nicer parts or check if introducing an extension method would be justified... But the benchmark is the most surprising part of this video. It raises more questions about how well the compiler actually optimized. I don't want to be questioning code style for such performance differences.
@evancombs5159
@evancombs5159 Жыл бұрын
I think in this very specific case you are writing a method that you'll write once and never look at again. The task is a simple one so it is unlikely to ever need debugged. Since this is something that could easily be used all over your code base, I think the preference should be for optimized code.
@BoJaN4464
@BoJaN4464 Жыл бұрын
I definitely prefer the bad example, it's easier to step through and debug in my head as if I'm running the program on my squishy hardware. I process the "Good" example all at once which makes it difficult to actually know what is happening on a step-by-step basis and can make debugging a chore in more complex scenarios.
@CallousCoder
@CallousCoder Жыл бұрын
Clean code is a religion! I love line 2 but I don’t care they, literally read the same to me, I just prefer less code, less lines and short lines - as long as they don’t hinder speed (which is very rare). It’s all an opinion and a style choice. If you can code it doesn’t as you can read anyone’s code. What is simple for me is: if (n & 1 == 1) vs if (n % 2 == 0). They both do the same thing and are equally long and is totally subjective, except that my logical and in an interpreted language is about 35 to 94 times faster. Same with n = n >> 1 is in interpreted languages so much faster than n = n / 2. I prefer bit wise operations over maths. But a good dev can and should be able to read them both. I do not stress about it when people use divides or multiple lines, it’s a style that suits them, like a choice of words and accent are. Unless it slows down significantly then we’ll refactor it. I’m more worried about massive useless allocations- and context switches. And the primary thing I look at is at poor error checking. That really harms systems.
@MaxLaurieHutchinson
@MaxLaurieHutchinson 6 ай бұрын
In the realm of software development, YKD principles - YAGNI, KISS, and DRY - lay the groundwork for clean code. Beyond these, the art of great coding is achieved by minimizing cognitive load, enhancing readability, and simplifying the process of debugging and testing. Great coding isn't just about writing the fewest lines or creating the perfect YKD example; it's about ensuring the maintainability of that code, making it not only functional but also enduring and comprehensible.
@ozsvartkaroly
@ozsvartkaroly Жыл бұрын
Since there is no NET built-in way for this, we need a custom solution. My approach is simple: since this is a very simple feature, I wouldn't look for nuget packages, but just create and extension method for string that does the job, and for implementation I would choose the best in terms of performance. (And of course we need unit tests for that.) I think this is the best solution, because it gives you the clean code (abstraction through the extension method so you will probably never have to go inside the method) and the best performance too.
@KNHSynths
@KNHSynths Жыл бұрын
As a senior architect/developer, I only know a few rules: readability of code by people of all skill levels (because teams are not homogeneous), maintainability and reduction of technical debt. The rest is pedantry and nitpicking. Good code must be readable by all team members, from the weakest to the strongest (and by all those who come to replace them), it must be easily maintainable and therefore testable (separation of concerns), and it must reduce present and future technical debt as much as possible. Fashions have no place in development, and alas, many developers behave either like fashionatas, or like frustrated show-offs. That's why good developers are rare... And so is good code.
@Katniss218
@Katniss218 11 ай бұрын
I feel like the specific example picked in the video is not a good example of bad one-liners. How I read it is "you have an input, which you reverse, and convert to an array". Very simple sequential code, no parameters, no nothing.
@cocoscacao6102
@cocoscacao6102 Жыл бұрын
Here's my take: public static string Reverse(string input) { var rChars = new char[input.Length]; for (int i = input.Length; i > 0; rChars[input.Length - i - 1] = input[i]) { i--; } return new String(rChars); }
@kidsam27
@kidsam27 Жыл бұрын
How about using a stringbuilder ? It may sacrifice on readability, but from my limited testing it's twice as fast as your fast approach. private static string Reverse_Builder(string input) { var builder = new StringBuilder(); for(int i = input.Length - 1; i >= 0; i--) { builder.Append(input[i]); } return builder.ToString(); }
@BrankoDimitrijevic021
@BrankoDimitrijevic021 Жыл бұрын
Both functions are incorrect: they reverse a sequence of UTF-16 code units, not a sequence of “characters”. If two adjacent code units form a Unicode code point, you might get unexpected result. For example: 𝐀 ("Mathematical Bold Capital A", code point U+1D400) is encoded in UTF-16 as a surrogate pair of code units: U+D835 and U+DC00. So calling this method on string "x𝐀y" will produce "y\udc00\ud835x", instead of the expected "y𝐀x" ("y\ud835\udc00x").
@janedoe6182
@janedoe6182 Жыл бұрын
Why not just "input.Reverse()" instead of "new string(input.Reverse().ToArray())" ?
@numidium3
@numidium3 Жыл бұрын
The second one is more functional (as in the paradigm). Some people prefer functional programming but I'm more of a procedural guy so the first one looks better to my eyes. I think it comes down to your preferred paradigm.
@Ashalmawia
@Ashalmawia Жыл бұрын
I would write a .ToString() method (maybe with a different name) that works on char arrays and then I could write => input.Reverse().ToArray().ToString(); if you wrote one for enumerables of char, it could become => input.Reverse.ToString();
@ozsvartkaroly
@ozsvartkaroly Жыл бұрын
04:19 - next time I recommend using Rider's introduce local refactoring action, you can do it in very few keystrokes, its faster, simpler, less error prone.
@petervo224
@petervo224 Жыл бұрын
When Martin Fowler lists the Refactoring techniques in his book, each technique is always coupled with another as its opposite, e.g. Extract and In-line. What is shown here is an example of In-line expression refactoring, and if doing the opposite, it is the Extract. As Refactoring's purpose is to make code design simpler in various aspects (flexibility, readability, debug-ability, etc.), performing a technique itself, e.g. In-line here, does not means serving that purpose without the context and scenario it is applied to. So I full-heartedly agree with Nick when pointing out the Linked-in post, as the post's author seemingly assume In-line refactoring into 1 line is a silver bullet for simplicity. Personally, I enjoy chaining methods in Functional paradigm since it can lead to nice readability, but I don't want to limit within that, and when opportunities present, will still perform Extract of the chaining later either for debug-ability or reusability. P.s. I personally think the author's in-line/chaining still does not improve much of readability while sacrificing debug-ability in vain. I would prefer sth like "return input.Reverse().ToArray().ToStringFromCharArray();", not "return new string(input.Reverse().ToArray());". If you mean to chain, at least chain all the way, not putting the chain as a method or a constructor's parameter.
@MatheusAugustoGames
@MatheusAugustoGames Жыл бұрын
I'm okay with more than one dot per line, as long as that's the only thing happening in the line. For instance, I'm okay with: return input.Reverse().ToArray().toString(); [I'm not sure if there is such a method, I'm not from the .NET bubble -- I just love your content!]
@adambickford8720
@adambickford8720 Жыл бұрын
I see comments around debugging; is this a Visual Studio limitation? If i have something like this in intellij: `System.out.println("Hello %s %s".formatted(title(), name()));` All i have to do is alt+click the string literal and it will call the functions and show the result of `formatted` in the debugger. If i'm in a loop or whatever where i don't want to click over and over i can simply add a watch point for the expression. You can also trivially set the breakpoint to within the lambda definition vs lambda execution. Am i missing something here? By 'debugging' do people really mean console logging?
@XeroKimorimon
@XeroKimorimon Жыл бұрын
For me it's about redundant variables. For short functions like that, doing something like "var reverseString = input.Reverse();" is like the same thing as putting the comment //reverse string array I mean yea, with all the context given to me, I can clearly understand that it's the reversed string. Having that extra name is redundant and puts more cognitive load for me. Personally, I only make new variables if any of the following get checked: - The formula used would not explain the intent of variable usage, ex: var size = x * y; - Caching results of expensive computation - Maybe if using the results of cheap computation multiple times, say like a simple getter.
@DemoBytom
@DemoBytom Жыл бұрын
So only focusing on the initial "inline things" approach, and not looking at perfomance or the fact you can't reverse unicode strings like that at all. Only focusing on the very "rule" they wanted to impose here. I do actually kinda use that, sometimes. Looking at the examples neither is necesairly more or less understandable to me. The second one might even be easier, because I read left-to-right what's happening. ReverseString_Good: "I'm creating a string, that is the input reversed and turned into an array because that's what the constructor expects". ReverseString_Bad: "I'm creating an array. I'm reversing the array. What was it - oh it was the input. I'm creating new string using the array. What was it? Oh it was the char array. But it was reversed.. etc". Generally looking at that code I have to jump between lines back and forth to figure out what is happening, and what is being done. In THIS example it's simple enough to not be a problem, it all fits in the 3 lines after all, but when the code is more spread out, or methods are longer, Linq's have more steps, it becomes a little bigger pain. So personally might use the second approach but format it differently: public string ReverseStringGood( string input) => new( input .Reverse() .ToArray()); This is concise, you can read it top-to-botton -> left-to-right in the order of operation that matter while creating the object that you want to create. If the constructor is rarely used/obscure I might go with prefixing the parameter name, but in case of new string, I probably wouldn't, as it makes no difference, and should be obvious enough: public string ReverseStringGood( string input) => new( value: input .Reverse() .ToArray()); Now I recognize there is a big problem with that approach - debugging. It's hard to get the intermediate steps while steppting through that method. And this is what I'd consider first - am I still developping - I'd probably write it more "debug" friendly first, before I'm sure through both debug and unit tests, that it does do what I want it to do, before inlining. Can I inspect values other way, can I step into methods to get the intermediates etc. In this example I might not care, since it's trivial anyway, but in some cases I might choose option A over B, or vice versa... In the end I guess, it's mostly common sense approach for me, more than just strict rules to follow blindly. But the problem here, for me, is that showing a simple example that would fit on a tiny screenshot, to be shared on social medias, always means that those example are so trivial it makes barely any difference. And people get hung up on "well you can scrap it, and do it COMPLETELY DIFFERENT WAY" etc, instead focusing on the original idea.
@asteinerd
@asteinerd Жыл бұрын
One-liner; My brain just understands fluent-interfacing just as well as the decomposed method that uses "one dot per line". HOWEVER, but I'll only ever write that way in code I know I own and other hands/eyes will never really be in there. So I like to break down the methods into verbose line-by-line processing for the sake of any Junior Devs that might have to adapt the code.
@pinguincoder
@pinguincoder Жыл бұрын
"Premature Optimization is the root of All evil" I personally always opt for the variant with the best readability and the least complexity. If the context requires it the I will optimize for Performance afterwards. 1. Make it Work 2. Make it Good 3. Make it fast ( if necessary) There is just no point for me optimizing for nano seconds if it is not required
@zabustifu
@zabustifu Жыл бұрын
I'm too lazy to benchmark anything, but the "bad" method uses Array.Reverse, which is probably much faster than the Reverse extension method that the "good" method uses, as it works with any IEnumerable and may not be optimized for arrays. That's probably the main reason for the bad performances of the "good" method. The ToArray() call probably doesn't help either.
@MilYanXo
@MilYanXo 11 ай бұрын
Come on, while I get your point, again, if "input.Reverse().ToArray()" confuses anyone, they are in the wrong line of work. Which doesn't mean long multi-level sausages should be tolerated. As for the rest LINQ has always been a marriage of convenience, not performance, if performance is the issue because you have to reverse bilions of strings, you would most certainly use the _Fast approach, or even go lower into unsafe for those extra few nanoseconds. :)
@philblandford5560
@philblandford5560 Жыл бұрын
input.Reverse().toArray() - reverses the input and then converts it to an array. Does what it says on the tin, in one line. What am I missing? Who gets confused by this? I'd have to look at the first one for longer to work out what it's doing, the second one reads almost like English. The first one, we have to know that charArray is mutable, and that the action performed by Array.Reverse affects it in place (and also, we now have to have some idea of what the Array class is and what its static methods do). Also, we've introduced the concept of a CharArray, which we didn't need to know about when just calling the Reverse() method on a string. That said it's weird that C#'s Reverse method doesn't just return another string - Kotlin just does "My String".reversed(), and then none of this would even matter.
@larsconrad2669
@larsconrad2669 Жыл бұрын
what do you want to achieve with 'clean' code? I'd like to focus on writing code that's readable. Understandable. Almost like a story. Some code parts require detailed descriptions, with declarative function and variable naming, grouping the right lines of code together. and giving space to other parts of the process. Other times, eg. when going through a list of early returns, i dont want all n '{ return false; }' blocks occupying three lines each. I tuck'em inline behind their ifs. Similar pieces of code should be recognizable in style on first glance. I believe that no discrete rules, when followed verbatim in all situations and regardless of context will ever produce readable code. But I understand that everyone has their own reading style. And formatting code 'smart' can't be formally checked. It'll stay a hard problem.
@padonker
@padonker Жыл бұрын
IMHO performance trumps readability trumps dogma. Simple. And for this specific example: if you want to debug and everything is on one line you've got to jump through hoops to examine how your code affects data.
@romanpelikh1862
@romanpelikh1862 Жыл бұрын
Using one-liner methods isn't always the optimal approach. It doesn't necessarily align with the KISS principle. At times, splitting code across multiple lines can be more beneficial. Debugging becomes simpler when examining log files. For example, if an exception occurs on line 36 and that line features multiple chained methods, determining the exact location of the exception can be challenging without running the app with a breakpoint. Simplicity doesn't always mean brevity ;-)
@Yves_Cools
@Yves_Cools Жыл бұрын
@Nick Chapsas : would you have "policed" this linkedin post if the second image had the .ToArray() call on another line, properly indented with the .Reverse() call ? Because to me this seems to be basically a matter of personal preference if you want to split it up or not. Would you also "police" linkedin C# .NET posts that put the starting curly brace on the same line as the method declaration ?
@joshpatton757
@joshpatton757 11 ай бұрын
I don't see the dot chaining as a problem, that portion is very readable to me. Each dot represents a sequential step in a nice clean predictable sequence. Taking that dot-chained expression and making it a parameter of the string constructor is where it becomes less readable. It's not the method chain that's the readability problem, nor is it the parameter, but nesting one inside the other. If I had been handed the the problem and asked to make it "clean", I would have probable wound up with: public string ReverseString(string input) { var reversed = input.Reverse().ToCharArray(); return new string(reversed); } The method chain on the first body line is nicely readable, representing a simple sequence. The return line with a string constructor is also pretty straightforward. This is assuming we don't need to deal with any culture/locale considerations like accent marks. In a real world situation, I'd probably be using a string enumerator to break into a collection of grapheme clusters, not chars, which would be less readable but more robust unless I could absolutely guarantee that it isn't needed.
@leetaeryeo5269
@leetaeryeo5269 Жыл бұрын
The ReverseString_Good is fine to me and what I would do, but that’s based on how I came to coding. Chaining functions isn’t really difficult for me to parse. It easily reads as “take a string, reverse it, and give it to me as an array, then make a new string out of it”. That’s not to say the “bad” approach is something you shouldn’t do. Everything except the fast one is pretty much equally easy to read for me. I’ll admit, I’m a bit surprised that the “bad” code is so fast compared to the one-liner, considering how similar they are conceptually. But that level of performance difference isn’t necessary for my use cases.
@azcodemonkey
@azcodemonkey Жыл бұрын
I’d likely put an extension in play to make it more readable, e.g. input.Reverse().AsString(). It’s readable to me, but i wouldn’t complain if it were the “Bad” in the code base. Maintainability is more important than my ego.
@WPJName
@WPJName Жыл бұрын
we can rant even further over that with public string WtfReverser(string input) => new(input.Reverse().ToArray()); And just pray for yourself in two weeks while you will try to read this code to understand wtf is going on ;-)
@kylekeenan3485
@kylekeenan3485 Жыл бұрын
The 2nd example is terrible as you say because of the cognitive load, the first example was a bit wordy. My favourite was actually your change over 2 lines, thats how I would have written it. KISS is important but an action on an action on an action is hard to read. An action on an action seems to be a reasonable maximum for keeping things simple. But never more. The first example was individual actions too simple and overly wordy, the second was 3 actions in 1 which is hard to decipher. Your example was both short and easy to comprehend. That was KISS imo.
@JordanBaumgardner
@JordanBaumgardner Жыл бұрын
No, I always make this point in code reviews, the second is cleaver but not "readable". If they broke it up on to separate lines, maybe. But I would always opte for the first one. I (57m coding for cash since 17) HATE trying to figure out what you were trying to do 2 years after you left.
@harald4game
@harald4game 11 ай бұрын
I'm not a fan of clean code. Instead I consider comments as code. Clean code and fluent style are two things that produced highly undocumented code. So for a clean code I only would take the versions with extra variables, because clean code tells us to name variables and a dot in fluent style is a variable. For code that is executed millions or trillions of time I'd use the Nick version in execution. I would add a comment with the good version maybe. I'd rather have 20 lines misplaced half invalid comments than no comment on a equals followed by 10 or 20 dot connected fluent style functions, shuffling around multiple types with hacky usage of functions so it can be this single equals. Introducing the term CO2-Aware-Programming, where I mean that a developer also needs to take responsibility for power consumption of the code in use.
@Kotz_en
@Kotz_en Жыл бұрын
I don't mind chained method calls when it's just two methods as shown in the example in the video. So in this example in particular, the "good" code is indeed more readable for me than the "bad" code. If there are more chained calls, then I would break them down into multiple lines, like people usually do when they are using Linq. But I would still not create variables for intermediate states, unless they could somehow improve readability of the code. This is all about style though. People who like functional programming probably favor chained method calls, whereas people who prefer procedural programming favor creating variables and calling functions in a step-by-step manner. There is nothing wrong with either approach, and I wouldn't block a pull request in a code review if someone were to favor one style over the other. What bothered me in this code is that I immediately noticed their performance would be terrible. And your benchmarks proved me right. Now that would be a good reason to block a pull request.
@perj6628
@perj6628 11 ай бұрын
I love this example of reversing a string. It made me think... The two functions dont do the same thing even though they return the same thing. Array.Reverse() in the first function is not the same as String.Reverse() in the second function. In this instance, i prefer the bad one. But I believe that the second function simply could be returning input.Reverse() and skip the .ToArray and the new string() parts. 😊
@Bliss467
@Bliss467 Жыл бұрын
i think one line is always _potentially_ more legible. in kotlin, it's `input.reversed().toString()` which _is_ more legible. i agree. i hate nesting expressions inside of constructors or function calls. in c#, i would do what you did.
@RichLee_laughingblade
@RichLee_laughingblade Жыл бұрын
Heck, I find this irritating tbh. If perf is critical then ok, use that. Otherwise I don't care. This is likely one operation in a much bigger story. I might have UI, and API, some pattern stuff, tests to write, maybe a pipeline to tweak...
@MirkoVukusic
@MirkoVukusic Жыл бұрын
I think this is a great example how subjective it is, and that there is probably no point in making rules here. For me, chaining methods is really readable. I'm not sure I can explain why, but for me it reads like a sentence (which is not too long). Multi line approach is more text, more variables to read and memorize which equals to more mental load. I can also write it faster. Code is more compact so less scrolling. At most, I would write a 2 liner.... 1st line all the chaining and the last line return new string(reversedArray), but for only 2 dots, probably not.
@klocugh12
@klocugh12 Жыл бұрын
3:00 without watching further, no. I'm guilty of one-lining all the time (mostly Linq combos on collections, also => for methods), and it makes it pain to debug not having it in steps. KISS principle comes with unspoken follow-up *but no simpler than necessary*
@failgun
@failgun 10 ай бұрын
I prefer the "good" example in this case because it's simple and the code reads semantically left to right. Reading it makes me think "this method return a string, okay, that is the input string, okay, but reversed, okay, and converted to a char array, oh because that's how the string constructor works". It communicates that the intermediate states of the string are not important to understand the intended result. The version with multiple assignments, to me, is harder to read since I see "oh, you've created an array, why? is that important, do you need it later?" - this "clean code" tip seems to be "don't define temporaries if you only use them in the next line, and that's the only time you'll use them" which I _mostly_ agree with (and I know the actual assignment will be optimized but the way you understand/debug the code isn't optimized). I take the point regarding the "bad" one being easier to debug, but the "good" one wouldn't be _hard_ to debug, would it? Obviously for more complicated examples (especially if the temporaries are being passed as function arguments in a one-liner instead of method-chaining like this) defining temporaries can be clearer, but not in this case, in my opinion.
@metacob
@metacob Жыл бұрын
The most valuable takeaway for me was how to reverse strings fast. As for which example is "simpler" or "more readable"... that's very subjective in this case. I might go as far as to make it an expression method because what actually happens in there is very very boring. Making it any more than one line just wastes a lot of vertical space you have to scroll/scan over. When there's some actual business logic going on, or some non-trivial computations, I totally agree - use super descriptive temp variables, never do more than one thing in one line and so on. Now you could argue that mixing "styles" like that hinders readability too and that might be true for lots of people as well. Thanks for always putting stuff to the test performance-wise though. I'm not a fan of nitpicking style questions, but long discussions where people just "guess" the performance of something are extremely tedious and I love that you shut those down before they even happen.
@cew182
@cew182 Жыл бұрын
Looking at this as a concrete example, I would inline it for a couple of reasons. There is less code which makes it faster to read. Maybe I'm just highly familiar with these methods but I understood the inlined code instantly, knew it was using basically reliable calls and I didn't consider debugger inspection something I would be likely to do on it. If I did it's not hard to quickly pull things out into a variable, at which point I would leave them since I obviously had to do it once. Looking at this conceptually, Nick is 100%. If inlining makes the code too long it becomes too much to understand. As the complexity goes up then I think you want to break it down both to make the complexity understandable and because you're more likely to need to debug it at some point. Some of what makes the difference for me is if it's .net code being called or something from a library or your project. I assume that .net has been heavily debugged and used, in most cases, and operates predictably so I'm less inclined to feel .reverse or .toArray will do something unpredictable. Unless I'm calling the wrong thing in the first place which should be pretty obvious by the input and output of the method.
@svetoslav.hadzhiivanov
@svetoslav.hadzhiivanov Жыл бұрын
In my opinion, the "bad" example is more readable and more importantly debuggable. I've been there when Rider suggests to me an optimization of a foreach loop into "foreach (var item in items.Select(x => x....)" but that didn't seem more readable to me.
@muhammadtariq8323
@muhammadtariq8323 Жыл бұрын
Why don't you share a video to compare different framework's benchmarks ? .NET, Python, Java, Go for web development etc?
@codingwithgyver1637
@codingwithgyver1637 Жыл бұрын
For me, one-liner code will not show you the detailed part of logic that cause failures. For example in your method "ReverseString_Bad will tell you which line of your code causes nulled values or if that line will not properly reverse. If ReverseString_Good don't show the expected output, you cant tell which part of your method calls causes failed reverse string? Also setting a breakpoint would not help you to pinpoint which of your method calls failed to show the correct result
"Don't Use Fields in C#! Use Properties Instead" | Code Cop #003
10:27
Stop Using FirstOrDefault in .NET! | Code Cop #021
12:54
Nick Chapsas
Рет қаралды 76 М.
когда не обедаешь в школе // EVA mash
00:57
EVA mash
Рет қаралды 3,6 МЛН
How I use Cursor AI in my new project - It's insane 🔥
3:34
Fathy & Abusrea - فتحي و أبوسريع
Рет қаралды 81
"Your Code Has a SQL Injection!" | Code Cop #007
12:11
Nick Chapsas
Рет қаралды 51 М.
The Logging Everyone Should Be Using in .NET
15:34
Nick Chapsas
Рет қаралды 71 М.
Don't Use Polly in .NET Directly. Use this instead!
14:58
Nick Chapsas
Рет қаралды 63 М.
The Pattern You MUST Learn in .NET
20:48
Nick Chapsas
Рет қаралды 87 М.
"The readonly Keyword Is Useless!" | Code Cop #012
9:42
Nick Chapsas
Рет қаралды 35 М.
"Stop Using Async Await in .NET to Save Threads" | Code Cop #018
14:05
The Correct Way to Delete Data in .NET
11:11
Nick Chapsas
Рет қаралды 12 М.
Why You Shouldn't Nest Your Code
8:30
CodeAesthetic
Рет қаралды 2,7 МЛН
What is Span in C# and why you should be using it
15:15
Nick Chapsas
Рет қаралды 255 М.
когда не обедаешь в школе // EVA mash
00:57
EVA mash
Рет қаралды 3,6 МЛН