Hacker News Reader

Comment by panic | Hacker News Reader

Lots of short functions are usually worse than a single long function. When you go to figure out what the code is doing, instead of reading through each statement of a single function and understanding it as you go, you keep being interrupted by new functions you have to read and understand. What does "reduceDepositedCoinsBy" do? Yes, I'm sure it subtracts its argument from the deposit amount, but what else does it do? Could it be causing the bug I'm trying to fix? No, it just does the obvious thing, OK, now what does "removeFromInventory" do? And so on…

View on HN

I agree. Obviously 'god functions' and 'god classes' are bad, but splitting up tightly coupled, related logic into piles of <10 line functions seems ludicrous. Not only does it hurt readability (and debugging) like you mentioned, but it also promotes false sharing of functionality. By splitting things up into very small functions many things may start to appear reusable. The moment it turns out one or two instances of the code that calls all these small functions need to have subtly different behavior you'll be hunting down each and every piece of code that calls the function, to be sure that changing it won't break completely unrelated parts of the code.

So, leaving aside the fanaticism around number of lines a functions should have which I disagree with, there are some big advantages of splitting a functions in logical units.

1. If you have ever written unit test for 300 line function you know it gets dicey. Such a function will likely require too much setup and will require complex mocks/patches/doubles to be unit tested.

2. As I mentioned before, I don't believe in number of lines thingy but logical unit of separation. If a function, opens a file, reads & parses config data from it, reads URL from the config data , parses http response and writes updated bits to config data - these distinct things can be logically separated and will be easier to unit test perhaps (again production scenario might be more complicated). Also, with modern IDEs and editors jumping back&forth between functions should be a breeze and your IDE/editor should provide an outline of file which can be scanned quickly.

3. If you have written a long function that does too many logical things(not again by number of lines) - next person who makes changes to it, isn't going to break it up - even if breaking up makes sense. For example, lets say in above example original code was written with expectation of certain format of config data but now somehow the function has to support more than one config data. Now the code is so tightly coupled that - it is harder to break out parser of this new format to new function and hence the function will keep growing (and did I mention the unit test setup has to support new config format as well somehow).

3. If however, lets say method computes moving average of a timeseries, it probably makes sense to have it all in one function even if it becomes slightly longer.

TL;DR - Cargo culting must be avoided at all costs, however we don't need to throw the baby with the water. Each problem calls for our own judgement. Separate the functions by logical units, rather than number of lines, use proper editor/IDE.

> Each problem calls for our own judgement.

Indeed it does, but if you have never even tried to go full "Uncle Bob" and write really small SRP functions and classes, then your judgement won't be well-informed. You'll be surprised how well it can work if you've never tried it. It works for readability, maintainability and testability.

Statements like the grandparent's "Lots of short functions are usually worse than a single long function" IMHO mostly show a lack of relevant experience.

Let me come out and say - I think OP is wrong. When people write 500 line long functions the only way they are going to test it is, run with real input and see if it breaks.

Such testing is not only insufficient, but any regression that is added as a result of a change will go unnoticed too, until a real user hits that bug. You will notice how most people who are supporting long functions - don't even talk about how the heck they are going to test it.

>Cargo culting must be avoided at all costs, however we don't need to throw the baby with the water.

But at the same time, just because you can transform someone's point into an ultra-generic form that's true, doesn't mean their original point has any validity. When someone gives you bathwater, don't spend too long looking for babies that aren't actually in it.

You have gone wayy too meta for me to understand what you are saying. OP seems to make a claim that more functions hinder readability which is not true if you are using proper editor or IDEs, on other hand smaller functions enable unit testing and extensability which are pretty worthy goals if you are working on a project with multiple people.

Its good to keep functions from becoming huge.

But when the article says that a function with 10 lines of code is too long and needs to be split up into a bunch of 3 line functions, that is a very extreme viewpoint. It's okay to flat-out reject such a viewpoint. It's not throwing the baby out with the bathwater.

Let's say I tell you that you need to cap your car's speed at 15mph to save gas. Don't go around telling people that "well, if you control your driving you can save gas, let's not throw the baby out with the bathwater". Just tell people I'm being dumb and to ignore me.

If the only thing true about a statement is such an extreme generalization that the generalization bears little resemblance to the original statement, just go ahead and reject the statement.

(Note: It's okay to agree with the article's view, that functions should be extremely short. This is only advice for disagreeing. I'm sorry for making my analogy intentionally ridiculous.)

Yes it's more cognitive overhead but it's a more functional than imperative style which usually has many advantages. If the functions are named well (naming being the hardest problem) then it should be easy to read without knowing exactly what the implementation of the function does.

I fail to see how reduceDepositedCoinsBy is in any way more functional.

Reply on news.ycombinator.com