In this article, we’ll Learn a simple principle that can help you refactor your code more effectively “The Five Lines of Code Principle”. This prin...
For further actions, you may consider blocking this person and/or reporting abuse
I'm sorry but the original function is fairly easy to understand, and 10 lines is still small.
Your end result is less understandable IMHO, more cognitive load. Maybe because I'm not proficient in this language.
In the end, a developer spend 90% of his time reading code. That is all that should matter : readability for maintainability.
+1 that the original code is easier to read. I think this is a classic article of advice that has no fundamental justification. Five could be three or ten and the rule still doesn't make sense; I think the actual issue is the metric.
An alternative would be time for another person to understand the function. If another person can write the pseudocode for an existing function within X minutes of reading, it's a good function. Extra points for not having many comments (b/c comments don't run, code runs, so self-documenting code is more trustworthy than commented code).
We mainly agree, but I have to disagree on the "no-comment" dogma. The code, as perfect as it can be, gives the "how". Sometimes, and quite often, I have to give the "why" in comments, for my readers and my future self.
Agreed, I prefer an economy of comments rather than no comments.
Agreed for the why part, it's help for our future-self as human tends to forgot what we've done in the past (that is not my code 🤣)
I am in the comments at the dependency boundaries, where you are using code from a library and you need to document why your adapter code is needed. I would also like to point out that logging to telemetry instead of comments can add more details on the why of that section of code while ensuring that you would need to update the telemetry values when you alter the code. Most of the time when I see comments, they are reiterating what the code already tells me in the first place and add no value.
The tools out there already generate documentation based off of the function/method and the only other helpful comments would be examples of using the function/method, that is the best use case for adding them.
I agree that comments muddy the waters with unnecessary details that can be made irrelevant, if the team is focused on pushing features, and it turns into additional tech debt. But this statement depends greatly on the team's process and if it is part of their procedure to ensure comments are always relevant, I have no objections to using them.
I agree that shorter functions are easier to read, but I'd be careful about this one.
Converting RGB to HSL isn't an overly complex algorithm, but it takes more than five lines. (search for the function
rgbToHslin the linked code). I'm not sure if the five line principle would make it easier to more difficult to read.I feel that this might be true for an implementation of something more complex too, e.g. the SIFT algorithm.
When implementing a complex algorithm, the nature of the algorithm means that there will be a lot going on. To condense everything to five line methods would mean either:
I also agree that splitting into too many small functions makes code harder to read. Instead of reading through the code's actions in sequential order, you end up having to jump up and down through your code to see what's going on.
While functions should generally remain short, targeting a specific line-count seems overly simplistic and I wouldn't recommend it as more than a rule of thumb for new developers. And even then, 5 seems excessively short.
Completely agree that having to jump up and down is distracting. I personally target the height of the screen as an approximate maximum length. Even then there are some complex functions that overflow. But if breaking them up makes them even more complex, I leave them as is.
Not the number of lines is important, but how well the code conveys your understanding of the problem it solves to the reader.
If an operation takes six separate steps, reducing it to five lines or splitting it into two operations for no other reason than that you want to avoid having more than 5 lines is not going to improve the quality of your code.
Thank you all for commenting.
I highly recommend you all to please watch Five Lines of Code • Christian Clausen • GOTO 2022 — YouTube reference video and Expert Talk: Five Lines of Code • Christian Clausen & Julian Wood • GOTO 2022.
I agree sometimes it is difficult to write code within 5 lines but the five lines of code principle is not a strict rule you can adjust accordingly.
If you want to see a real-life example please visit Rails GitHub Repository. The average all-method length is between 5~10 lines.
This article example is simple but there is a lot more you can do using this principle. That's why I recommend you all to please watch the
GOTO Conference Video, Christian Clausen explains it very well.I started watching the video; but then had to fast forward through a lot of it: I just couldn't bear to watch it 🤢
He's basically taken a sensible principle: "keep functions short" and turned it into a dogmatic rule with a completely arbitrary limitation. He gives absolutely no compelling reason for why a 5 lines maximum is best and then demonstrates what felt like a really convoluted process for refactoring that was simply too painful to watch.
As the comments on the video - as well as the responses here - make clear; the "Five Lines of Code Principle" is not something any sensible developer should adopt. Just "keep functions short" + DRY
It's not always possible to accomplish a task in just five lines of code, and there are certainly cases where longer code is necessary. However, by striving to keep code as simple and concise as possible, developers can improve the quality and maintainability of their code.Thanks for sharing
I have seen this mentality try to scale to over 10million loc and the end result is a code-base that is fragmented beyond comprehension with rampant duplication. Less we forget Goodhart's law: the moment a measure becomes a target it ceases to be a good measure.
I agree it's good to stick with small concise functions and maybe 5 lines is the magic number, but I don't think it should be a goal on itself. It's important to know why you want functions to stay small and you did a good job of explaining that in the article! 👍
I love the principle behind the article, as most code can and should be refactored to simplify the logic. We just can't forget the rationale behind the principle before applying it, which in the example or any blind application of such a principle may actually further complicate and hinder our code just as much as bloated and overly complex methods.
The rationale behind this principle is that it should increase the readability of the code. It's intended application is more like a heuristic than a logical fallacy, which for those that don't know formal logic this means that it is used more tentatively and not robotically because sometimes it is wrong. You could also think of it as a horizon one moves towards on a sphere, where you can never reach the horizon per se but after pursuing it for some period can look back and see the progress you have made anyway (much like most ethical codes that accept human imperfection).
Now the example you provided was 10 lines of code, double the number the rule calls out. However, if we first ask ourselves would comprehensibility and readability actually be improved to split that apart and thus remember one function of 5 lines while parsing another, I don't think most people would. I get the example was overly simplistic to demonstrate the rule, it just may be wise to note that and to mention the rule's motivating factor (modular, readable code) is something to keep in mind when writing and refactoring code as a sort of internal test to use prior to further breaking code down.
Another interesting exception to the concept is illuminated by the notion of turning every single line of code into a method and why that is not done. The reason we don't have each line wrapped in a method structure is because at that point the method structure code and the call to the method become unnecessary fluff that adds lines of code without being meaningful to us as humans reading it and often to the interpreter or compiler that already tosses out much that humans add to understand code (comments anyone?). Thus it is clear the purpose of creating methods bundling functionality is to wrap a related set of lines, functionality or object traits into a single structure we can call later without rewriting all of those lines over and over again. While a good rule of thumb to try to keep your methods below 5 or even 10 (15 if it pleases you, this number is somewhat arbitrary in importance ultimately) there are times this won't be possible and that's worth remembering before refactoring your codebase by this or another rule mechanically without attempting to parse the meaning of the rule.
Which I don't mean to be a criticism of the article really, just something to keep in mind. I loved the article and find your writing on the topic unusually pleasant to read as compared to most other articles I see on platforms intended for developers lately. Good work and keep it up!
I generally agree with what you have said but sometimes too many functions provide additional security risks and poorer performance. There is always a balance between purpose and style.
There's a tradeoff between conciseness and indirection. All else being equal, conciseness is good, and indirection is bad. But by increasing the conciseness of the function, you also increase the indirection: now the reader of your code needs to look in one more place outside the parent function to understand it. It's not too much of an issue in your toy example, but it would be much more of a problem if the original function had been, say, 25 lines — according to your metric, you'd need a minimum of 5 functions (probably 6 or more) to capture that functionality, which would most likely just be a mess of bad abstractions.
Indirection can be a worthwhile tradeoff if the abstractions are good, especially if the logic of the child functions is reused in many places. But indirection in order to meet an arbitrary metric will only make your code worse.
Imho, we can just focus on what part of our function that needs to be testable, in this article the calculate_item_price is what matter, the calculate_total (could be considered private function), and might unnecessary to test, and we can just skip this, the first part using for each are already enough, it's in between jump into many separate function and still concise.
caveats the one line (increase cognitive loads as mentioned in the other comments):
.reduce.calculate_item_priceThat's already many context switching in single function imo
This is excessive. Imo the defaults built into SonarCloud's cognitive complexity methodology are about right and stop you from creating an unreadable mess most of the time. Compressing everything into 5 lines of code means you will likely spend more time refactoring than you should with net zero or negative readability benefits.
You had me until the final step squashing into 1 line. Less lines is most certainly not the same as more readable!
This last part surprised me as you (@kanani_nirav) yourself explicitly point out "Optimize for readability".
If one of the arguments is that more lines is harder to understand, surely it can be inferred that 'more going on in a single line' can be too.
I would agree with the author that the original function is doing more than one task and should be converted into two, but step 3 was all the reduction necessary to fix the issue.
I would only take the calculate_total function and make it take another argument for a get_price function, that way, if there are different calculations per country, you can pass in the specific calculate_item_price function that you need and not have that code turn into a large mess.
Also, this makes for better unit tests, calculating total has a dependency on calculating item price and that shouldn't be a hard dependency. This is the whole point of test driven development, you find the boundaries of each unit because the tests are simple. The function given at the beginning could be simple enough that it wouldn't make a large difference, but there are many other examples where it would.
So, this article is giving the same advice as TDD, where you reduce the concerns of your functions by finding the smaller units, but it left the hard dependency to calculating total and that part is more important than getting the lines of code down to a specific number.
This may be the worst programming advice I've ever read.
I wonder how long it'll be before Chat GPT starts applying it 🤔
I'd love to see the author applying the principle on any reasonably large open source projects. That'd be a lot more convincing than just a simple 10-line example.
"Every function should be as simple as it can be, but not simpler!"
... is the golden rule of all ages.
5LOC violates the last part (sometimes), so it's not a good principle.
5 lines is too short, even for ruby. SRP will lead you to smaller line counts in general, but setting any hard number of lines is missing the point.
If you ever feel like moving multiple lines of code to a single line would improve readability, you're either doing it wrong or you're just a really bad developer
IMHO in 5LOC the following code is more readable:
def calculate_total(cart)
total = 0
cart.each do |item|
total += calculate_item_price(item)
end
total
end