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

Use code CLEAN20 and get 20% off the brand new Clean Architecture course on Dometrain: dometrain.com/course/getting-...
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/nickworkshops
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: bit.ly/ChapsasGitHub
Follow me on Twitter: bit.ly/ChapsasTwitter
Connect on LinkedIn: bit.ly/ChapsasLinkedIn
Keep coding merch: keepcoding.shop
#csharp #dotnet

[@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]

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]

that's a debugger/ide issue, not a code issue

[@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]

@@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]

"Debugging is harder" Only soy developers debug. (at least according to similar videos)

[@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]

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]

​@@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]

@@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]

@@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]

Also easier to see line coverage while writing unit tests

[@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]

Yup, and I think ideally you would do this as an extension method.

[@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]

this was very annoying to me to, never could ever explain me why things are better when they are in one line.

[@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]

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]

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]

@@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.

[@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]

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]

​​@@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]

@@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]

@@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.

[@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]

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.

[@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]

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

[@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.

[@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]

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]

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]

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]

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]

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]

@@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.

[@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!

[@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.

[@nitrous1001]

Code Cop sounds like a better name for a series since it sounds like an existing analyzer like Style Cop.

[@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]

Thats by far the best approach readability-wise

[@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.

[@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.

[@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 :)))))

[@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.

[@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

[@jongeduard]

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.

[@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]

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 :)

[@ymko2099]

Found it funny that the "bad" approach was much faster

[@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.

[@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]

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]

@@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.

[@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.

[@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]

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]

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]

@@jiebaef you can write entire programs in a single line in almost every language, it's not really specific to python

[@ara849]

Line through screen, line not worth.

[@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.

[@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.

[@MaxLaurieHutchinson]

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.

[@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.

[@alexandernava9275]

The second definitely breaks my favorite rule, one dot per line. Makes debugging and seeing what is breaking much easier.

[@lmao01]

This is like having a table with many different dishes and yet trying to compare 4 potatoes to each other.

[@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.

[@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.

[@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.

[@Jazz_10110]

Code readability is always appreciated rather than a chained single line code with a justification of "its more optimised" with no actual noticeable difference 😂

[@ronsijm]

Static analyzers usually give pretty bad advice on method chaining random stuff together. Mathematically putting everything in 1 chained line gives you a lower Cyclomatic complexity - so some people just blindly assume that is actually better

[@lolyasuo1235]

The first method is much cleaner than the second and the Nicks version. The reason is because everyone is used / has faced this version so it more readable.

[@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

[@Leacee25]

In this example, I think the oneliner is way easier to read because it reads like english. you have input -> reverse it -> put it in a new array.

[@sleeper-cassie]

But Nick! It’s still 1985, and hand-minifying my source code is the only way my employer won’t run out of disk space.

[@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 ?

[@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!"

[@failgun]

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.

[@tofu1687]

What ?!? There is a solution about string manipulation where Nick doesn't use span ?!?

[@dance1211rec]

Don't worry. The fast solution uses span!

[@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]

Except that he did use spans, the function takes a span of char, the string is just automatically being cast to a span. :)

[@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.

[@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]

Entire program one line... This makes me think of a book with no punctuation, no paragraphs, no whitespace, just a series of letters. :)

[@JoseFuentesLopez]

Once, a friend told me, 'I improved your code.' He rewrote my code into a single line. His lack of experience didn't allow him to realize that I had to debug that code to provide him with tested code

[@oscareriksson9414]

When I need to jump into an unknown project to do something, the simple code with more lines, fewer files and classes are always easier to navigate and I always appreciete them the most. The ones with the smart one liners and / or piles of abstractions and files just gives me a headache. When I need to fix something asap, I couldn't care less how smart the person who wrote the code was I just want to do the thing I was supposed to do, not solve some wierd code riddles 😂 I get it though, you do some thing and then realize you can do it in another way and then you realize you can do it even more smart etc. Feels good so you tell yourself it's "clean" but there is no such thing, it's just opinion. It should be about: does the code solve the real problem in a satisfactory way? Does the newest person understand it? If those are yes, then that's good code.

[@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]

Yeah. Exactly my case, too.

[@onman999]

I second this.

[@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]

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

[@mariastypsanelli7069]

Great job that you anonymize these posts Nick. For this particular example, I would mind the single line.

[@anantmishra6783]

Excellent video, these were my thoughts exactly! People make these wrong arguments about clean code all the time, I used to feel totally weirded out!!!

[@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]

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.

[@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.

[@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]

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]

@@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.

[@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

[@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...

[@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.

[@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.

[@victorpresti]

On the first example, I much prefer the “good,” compared to the bad. I find going through one line much easier, reads like a simple instruction. “It returns a string, which is the result of reversing the input and turning it into a new array.”

[@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.

[@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.

[@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.

[@zxcvbnmlkjhg8]

Using swap is the fastest way, swap head and tail. (Creating Span with stackalloc to handle it) And the only way to beat up above code is unsafe pointer.

[@ivanp_personal]

Multiple dots style leads to somewhat shorter code. I'd use this approach, however with formatting that puts every dot on a separate line. Also can use in a single line, when the code is likely to be "write once and forget".

[@codingbloke]

I tend to avoid creating unnecessary variables. This is conditioning from the old adage, "number of bugs is proportional to the number of variables". However, I now think that that adage is actually a result of having functions doing too much in first place.

[@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

[@YefoAkira]

The OO calisthenics do not talk about one dot per line. They talk about not going deeper within the class(module) with the idea of following the Law of Demeter. In that first example (reverse string), what happens is chaining methods, and is completely fine.

[@steff.h]

Hi, Why is String a part of the method’s name here? Do you not consider the parameter part of the method’s signature? When having to invoke this, do you prefer ReverseString(theString); over Reverse(theString);?

[@eesaaphilips9271]

very interesting. First time seeing string.Create; will definitely start using it

[@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 ;-)

[@MayronDev]

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.

[@spoonfuloftactic]

Fewer variables is simpler code. A single line with multiple dots is fine, as it reduces considerations for scoping, closures, etc.

[@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.

[@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.

[@JanVerny]

I don't know if this makes me the odd one out. But in the first example, I tend to prefer code that reads like a sentence. I like having single line ifs that don't break the flow, and I like not having to name useless intermediate variables. So when I read that a function returns a new string that's constructed from input that was reversed and turned into array, I find that better than reading three unconnected lines of code.

[@decapent316]

I like having single instructions used as Expression-bodied method (would apply to Reverse good, nick and fast). I like the syntaxic sugar of having less lines of code (no curly braces, no return statements either). I have no idea actually if it's faster or not. I would definitely need to Benchmark that. Any thoughts on the subject ?

[@MrNotour]

Keep it simple != Less character to right A code MUST be reable like a book with space, short sentence, one idea by paragraphe, argument align and wrap ... When you look at the code you should be able to identify directly the role of each element without thinking

[@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.

[@samnihao6943]

I prefer the first option. The second option looks like a more elegant solution BUT in case if I need to debug or add more specific logs in that code. The first one solution is better and it is easier to read. One action, one line.

[@emloq]

In personal projects I would use the single-line approach bc I like it and understands it, but that's only personal choice, in my work/real world I will use the first one

[@maxsnts]

There is two extra reasons to separate lines: Step by Step debug, and exception stack trace with code line of error. Sure the IDE helps, but is easier to debug if its separated. I use "one logical operation by line" guide line.

[@boltyk1]

if you have a lot of vars as middle steps, it will be harder to read. One liner is easier to read as you can literally read it as a sentence of a LINQ style query. Pointed debugging steps is a very rare case overall. If you need each step in debug - two automatic var extract refactors and you'll get it. Sometimes in the code simple chain operations are so much disintegrated to pieces with vars and names so it's very hard to read. Less code - less to read. Even make it a => instead of {}

[@mrwensveen]

I tend to use expression body functions to signal that it's a "pure" function (i.e. no side effects or in-memory replacements, etc.). I find expressions easier to understand because I know that the output follows naturally from the input. I don't need to remember that the contents of a variable or an array have changed, adding cognitive load. So yeah, I like the ReverseString_Nick version the best but I'd use an expression body: string ReverseString_Better(string input) => string.Concat(input.Reverse()); To handle unicode combine characters correctly, you'll have to use StringInfo.GetTextElementEnumerator: IEnumerable ToTextElements(string source) { var enumerator = StringInfo.GetTextElementEnumerator(source); while (enumerator.MoveNext()) { yield return enumerator.GetTextElement(); } } string StringReverse_Best(string input) => string.Concat(ToTextElements(input).Reverse());

[@hemant-sathe]

In the first example after reversing the string what’s the point in using the array & string functions?

[@ahmettasgoz8038]

Most developer think "less code or extensions work faster". In my opinion, behind idea of single-line code start with this.

[@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

[@Fireballl14]

I have a rule - if a line of code takes me more than 1 second to process than there should be two lines.

[@7th_CAV_Trooper]

More than 1 second to mentally process I presume and I agree if that's what you meant.

[@Kabbinj]

The reason it's hard to read is that you need to jump back and forth horizontally to follow the data. As long as the data flows in one direction, left or right, then chaining is not complicating anything, IMO

[@Raziaar]

I prefer conditional extraction too. Sure, it's another line of code (or more), but at least it's more readable, especially if you invert the negative conditionals to be positive conditionals. It's along these same lines.

[@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!]

[@wknight8111]

Code should be "readable", and what makes something "readable" is very subjective. But I find that, for native English speakers, code tends to be more readable when it has fewer control structures and can be read like prose: from top to bottom, left to right. Less indentation on the left, one method call per line, well-named variables and methods, etc. Name variables like nouns and methods like verbs or verb phrases, so you can really make things look more like prose. In your string reverse example, the first code snippet is more readable for this reason: Each line can be read from top to bottom, left to right. But in the second snippet you can't do that to understand it. You have to look inside the string constructor find out what you are creating a string from, and you don't know that the char array has to be reversed until you are halfway through the line.

[@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.

[@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))

[@daaa57150]

I like the "bad" version, for me it's the cleaner. Easy to read, easy to debug. For which one I would use in my app: probably your fast one if I found it somewhere, because who cares for such a small method in such a low level, if the internal is readable. It has a really readable name and that's the important thing, making higher level methods easy to read.

[@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.

[@HarshColby]

The rationale for the "good" code being good could be that "toArray" and "new string" are trivial constructs that our brains easily ignore. Thus, the only relevant part is "reverse", which captures both the intent and the approach to get the result. The "bad" example is wordy and forces you to read the whole thing to figure out what approach is being used.

[@lainwired3946]

The compiler is going to optimise both of them to basically the same funcgion anyway, the "wasted space" is only for the dev reading.

[@perj6628]

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. 😊

[@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.

[@chewbaccarampage]

Clean Code is context sensitive. There's an open letter between Jonathan Blow and John Carmack where John discusses in-lining all function calls in Rocketry code - not only for performance reason, but for avoiding certain classes of bugs. Now writing an application where all the code sits in the main() method probably doesn't work for business software, but in the case of Rocketry, it might be the "Clean" solution. I like how Rich Hickey put it, "Clean Code? Clean Code to whom?"

[@MrKlarthums]

I try to avoid evaluating complex expressions as method parameters. The nesting leads to poor readability and debugging. Splitting into separate lines fixes those issues and is more logically coherent: each line has a much more singular purpose and is read+executed in stepwise order before the method call is made.

[@MartinSGill]

Depends on your definition of "Clean Code". For me it means readable/maintainable code. In this trivial example I'd not be worried about either of those. In general though I consider expanded version "cleaner".