>I get the cold sweats when I see methods longer than a few lines of code
This guys code might not be as readable as he thinks. I do not want to read through your hundreds of 10 line functions because a few people said "long functions bad".
I liked the portion on naming. My God the amount of AbstractFactorySingletonViewModel garbage I see is astounding.
Simple anecdote: A colleague of mine had a really hard time getting some topologic sort to work. I kept finding issues with his implementations and he was getting frustrated so we aggreed to have a pair programming session. He wanted to put all the code in a single method (not terribly long, maybe 30, 40 lines), but the conditions kept nesting and no matter how much he tried to figure them out, he couldn't get them to work. I convinced him to extract two methods which would only be used once and only inside this sorting method. He was vehemently against it. After he finally agreed he realized the code was so easy to read, it was basically self-explanatory and obviously correct. No ammount of fiddling with the if statements could've produced that.
I don't impose fixed line counts for my methods. But when it makes sense, the readability improves a lot.
I have essentially the same story, so I won't repeat it. The big win was when my colleague seized on the idea of self documenting code. That was all he needed to write better structured code.
Small functions are not about readable code. It's about single responsibility.
Single responsibility is an engineering principle that is not limited to software development. It is the reason that we don't combine the breaking functionality in our car with the am/fm radio. We keep moving parts isolated because the fewer "things" that something does, the less likely it is to break. The less complicated it is.
But there are other side effects to creating small, single-purpose functions (or anything for that matter). It is not always obvious when you will have an opportunity to reuse something. And duplication is not always apparent. When you take single-responsibility as far as you can go, you not only isolate all of your moving parts but you maximize the opportunities for reuse.
And it goes even further than that. Your large functions likely have a few dependencies, at least. Those dependencies will make your functions more difficult to write tests for. And your test cases will be more complicated if you have larger functions because they are doing more than one thing that needs to be captured.
Readability is more about expressing the intent of code. You can do that in "long functions." You can express the intent of code while making it extremely compilcated. Hell, just add lots of verbose comments and your code will be more "readable."
Since you seem to be one of those people who has a stick up you about design patterns (your last comment about AstractFactorySingletonViewModel ... who hurt you?) I will offer you this piece of food for thought: the purpose of "best practices" and design patterns is to SIMPLIFY code. If you ever see a misapplication of them in the wild* then what you are witnessing is not "over" engineering ... it is POOR engineering. Consider that before throwing the baby out with the bathwater.
* I rarely do, so I often wonder to myself if this is a made up problem by lazy devs who don't want to actually study theory. But I do hear that this occurs from time to so I'll take you at your word that there are people out there that don't know how and when to actually apply design patterns properly. The problem is the misapplication, not the patterns themselves - which are just common solutions to recurring problems. Do you not think DRY is a good idea?
Ha! Thanks for sharing “enterprise FizzBuzz”; I’d not seen that before. I was digging through the code before I had a PTSD-related flashback from debugging some OO at my last company…
My friend and I were working together, when one of our seniors came over and asked us for help figuring out how to modify an algorithm that someone new on the project had rewritten. The algorithm was 20-30 well-commented lines of code, until the rewriter decided that it lacked flexibility. It had been rewritten to use a Singleton Factory to instantiate an implementation of an interface that itself delegated every snippet that could possibly be conceived of as a function to one of several different classes (Policy, Actor, Dispatcher, etc.). Every function was three lines or less, and all were entirely illegible taken in part or in whole.
I’m glad that team member got to show us how many times he read about design patterns and how much he knew about OOP, but it took four engineers (we later roped in another) to venture 15 layers down the call stack of virtual functions to figure out where a simple constant came from. Quality code indeed.
> Any rule, principle or pattern taken to its extreme is bad.
That's a non-sequitur. The word "extreme" describes matter of degree. If something is "good" then it does not follow, logically, that "extremely good" becomes "bad" just because it was "taken to the extreme." As you yourself said, context matters.
Then to say "it's always a balance", a balance between what? Good and bad? Simple and complex? What are you talking about?
We are speaking in the abstract, necessarily, when we speak of "small, single purpose functions" because we are taking the context out of it. Can we agree that the simplest solution should always win? If so, why?
After 25 years of developing software professionally, I have seldom seen someone apply design patterns, so-called "best practices" and engineering principles to a point where they shot themselves in the foot. It happens, we are all human after all, but in my experience it is not a huge problem.
What is a huge problem, is that time after time we inherit projects written by very decent hardworking developers who can get something that works out the door quickly but don't know how to write it in such a way that it is easy to change over time. We see massive files, massive functions, lots of cyclomatic complexity, terse variable names, duplication, tight coupling, near zero modularity and very poor separation of concerns. This is the norm, not the exception.
And yet lately, I have seen a trend that is a knee-jerk reaction against decades of work done to address this problem and it should be concerning. If 75% of the projects I worked on were tech-debt free, and we could identify actual examples of software where people wrote it so abstractly that it was incomprehensible then I would be on the same side. But what I see in the wild is not examples of people going "patterns happy" or taking things to a point where it starts to become complex rather than simple. What I see are developers who have never even heard of design patterns, or they learned one and then misapplied it everywhere because suddenly every tool looks like a nail to them.
Therefore my position is that we should be preaching single-responsibility and small functions more, not less.
This is a very level-headed defense of design patterns. I'm kinda like the parent commenter. I have a stick up my ass about these kinds of things. I think they're misused quite often. But it's good to see a reminder that they do have their place.
Even DRY, I've seen DRY cause test suites become hard to work with. Rules of thumb sometimes get pumped really hard and I think it causes people to over-use them. The rule becomes the goal rather than something more material like UX or DX. This code is DRY, DRY is good, therefore this code is good - I hear arguments like that quite often. But your take feels very grounded.
> We keep moving parts isolated because the fewer "things" that something does, the less likely it is to break. The less complicated it is.
It doesn't really work if I'm breaking up a function body into a bunch of smaller functions that will never be called from anywhere except the refactored function. I've just moved the complexity from a large function body into a bunch of smaller functions and the calls to them - but it's still the exact same logic, with the same complexity (but perhaps harder to read, due to all the jumping back and forth).
It’s a different kind of overhead. But I will take the tradeoff between ten 10 line methods and one 100 line method every time. I stand a much better chance of successfully modifying and testing the former.
Small functions are easy enough for me to deal with when they're basically library functions, but when they're split up/distributed in an OOP-y way I find navigating them and keeping track of them way more troublesome.
I think this is one of the more important distinctions, but you never seen it brought up. High level, business oriented functions almost never end up being more readable split up into a bunch of sub functions.
My second job had amazing code quality, and one that that always struck me was how the code was separated cleanly into layers that made the business logic layer super obvious.
It was 3 layers
- API interface (unpacking objects, etc)
- Business Layer
- DB/System Interface.
The business logic changes could often be totally contained, without updating the API or the database. It would be so obvious what layer needed to be edited, and the problems could be fixed so much faster. Rarely did a commit change two layers at once.
You can easily handle AuthN in the API layer, AuthZ in the business layer, and then access to a db in the system layer. Assume API is un-authZ’ed, assume the system layer is fully AuthZ’ed, etc.
That's fair if you only have 100 lines of code, but I don't think it automatically follows that the best solution for 10,000 lines of code is 1,000 ten line functions.
There's a tradeoff between understanding the function itself and understanding how/where the function is used (and therefore understanding what will break when that function is changed).
The best heuristic I've got at the moment is roughly the square root of the size of the module. So, for the 10,000 line module, my vague instinct is that I'll have one hundred functions approximately one hundred lines each. That's not a hard and fast rule, just an observation about tradeoffs and I would definitely expect (quite possibly even most) functions to differ significantly without losing sleep over it.
Agree - if a function is otherwise cohesive and doesnt mix independent concerns, I prefer one 100 line fuction to 25 4-line functions.
Splitting functions is tempting because it makes each individual function seem simper, but the higher cost in complexity and maintainability is not as directly visible.
Shorter functions are fine when appropriate, but length by itself is not an indicator of quality.
This guys code might not be as readable as he thinks. I do not want to read through your hundreds of 10 line functions because a few people said "long functions bad".
I liked the portion on naming. My God the amount of AbstractFactorySingletonViewModel garbage I see is astounding.