Why Swift Guard Should Be Avoided

> Has anyone seen production code with <=10 lines of code per function before?

I have seen code where a function was shorter than 10 lines. ;-) In that same code base, I have seen a function that was ~750 lines. And for that particular function, it actually made sense. It involved a lot of local state, and breaking it up would have involved passing lots of parameters to the new, shorter functions. (The average function in that code base was around 50-60 lines, I guess.)

Using a fixed number, like "10 lines" seems arbitrary to me. Sometimes it makes sense, sometimes it does not. It also ignores other factors, such as comments. Or readability. Sometimes, I 100+-line function can be marvelously clear, other times, a 5-liner can be so dense it makes my eyes bleed.

I have grown wary of "Always do this"/"Never do that"-advice.

"Try to keep your functions short. If they grow long, look if it makes sense to break them up into multiple shorter functions" is sound advice. But to do so without questioning or thinking is problematic.

My impression is that it is somewhat influence by the programming language. Code I've written have often needed so many variables to do its job that it is very hard to split out small functions. But when you can use closures and higher order function easily, I find it easier to keep the size of functions down.

But I prefer Linux take on function size:

> The maximum length of a function is inversely proportional to the complexity and indentation level of that function. So, if you have a conceptually simple function that is just one long (but simple) case-statement, where you have to do lots of small things for a lot of different cases, it's OK to have a longer function.

You might want to see this: https://www.youtube.com/watch?v=npOGOmkxuio Not really about production code, but still, some food for thought. Also this: https://www.youtube.com/watch?v=8bZh5LMaSmE

Maybe people just don't handle errors? Most of my functions are less than 10 lines, but then you add the error handling code, and it becomes 30 lines.

Also, it strongly depends on the code you are writing. Things like reading/writing data often end up as pretty long functions, while glue code between model and view is usually just a couple of lines.

My general rule of thumb is that a function should be small enough to be entirely visible on screen without having to scroll. It's dependent on screen resolution & font size of course, but I find that having to scroll down adds some cognitive overhead to understanding the whole of a function.

Following this line of thought I've become a lot more precious about vertical whitespace & I've adopted BSD style curly braces.

In my personal experience, the small function thing can drive a maintainer bananas.

When trying to understand code I haven't written, I have had the experience that trying to trace logic and debug through a bunch of little functions is more cognitive overhead than just seeing everything that happens within one larger function.

I get the idea that small functions are supposed to be wonderful, and clear, if everything is named really well and everything works as the name implies.

But for me the downside is it ends up feeling like detective work. I have to jump around and remind myself exactly how each little function works just to figure out what exactly is happening.

My dream IDE would let me see "expand" every function call ... like with little dropdown arrows .. so I could see it as if it were inline, and could trace execution linearly in one solid block.

> Has anyone seen production code with <=10 lines of code per function before?

It depends on the language. In Node.js code, it's not uncommon to not only see production code that follows this rule, but also puts no more than 1 or 2 functions in each file. Once you get used to it, and as long as you're quick with your editor/IDE's open shortcuts, I've found it a pretty nice way to organize code.

I personally find C code hard to follow sometimes, not because I don't understand C (I have more experience with C than I do JavaScript, if you count years), but because C tends to feature multiple functions per file, each with hundreds of lines of code per function. At least for me, that's hard to follow, and I hate having to move hundreds of lines up and down a file, even using Vim shortcuts.

I used to stick religiously to <=15, in C#. This was detrimental to understanding only twice, and a fairly small issue even then. Not a ridiculous standard for interns who don't have an excellent refactor-sense yet, so I didn't begrudge it back then.

Yes. Seen and written. Maybe it's easier with a small team, and it's certainly easier with a good language (Scala in my case) but it's absolutely possible.

The discussion is more fairly framed as: Do you use abstraction (hide details) in your code base? Number of lines of code is, perhaps, a symptom of code that doesn't use abstraction well, but isn't proof. We need to see the the code base, understand its design goals, and how it's actually used before we can pass judgement.

> Has anyone seen production code with <=10 lines of code per function before?

I've seen code where many functions were like this, and very few were over 30 lines. It was easy to read. And it was tested and generally working well. Naming becomes important, but that's part of the point of extracting functions for readability.

Likewise, agreed. Very weird to see this article score high on HN.

I've written 2 line functions already. It depends. If you write scala or any functional language and you do pattern match it's sometimes more readable if you have something like

    blabla match {
        case 1 => func1
        case 2 => func2
    }

instead of

    blabla match {
        case 1 => 
            println("1")
            1 + 2
        case 2 => biggerFunc
    }

To me the general idea of splitting a complex problem into a series of less complex problems is good. However the example used seems trivial and the solution is worse than the problem:

> 1. It is pretty long, sixteen lines, multiple parts separated by empty lines.

I wouldn't call 16 lines "long".

It contains empty lines for easier reading. Well done, not actually a problem.

But it lacks any comments on why the assertions have to be made.

> 2. It does several things. It gets an item by name, validates parameters, and it implements the logic of vending an item.

These are the things that naturally belong together. Separating validation from logic is not a very good idea. The logic requires the specific assertions in the validation part. They belong together.

> 3. It has several abstraction levels. The high-level vending process is hidden among the finer-level details like Boolean checks, usage of specific constants, math operations, etc.

In the single function implementation it's actually not hidden because it's all in one function. Use some comments to structure your function.

---

In my opinion separating this function into multiple functions would only make sense if these parts are used multiple times AND if they are commented well on why they need to act this way.

I feel the same. When I'm new to a code base its a pain to keep jumping around functions to see what they actually do. I agree functions should be short but this is a step too far for me.

I feel like somewhere between the two would pass. Keep the validation as distinct functions but inline validatedItemNamed, reduceDepositedCoinsBy.

This is an oversimplified example. When I imagine how both approaches would look like in actual production code with much more complex requirements, it makes much more sense.

Indeed. You should keep the complexity to a reasonable level, lines of code tends to correspond to it so it makes sense to keep the number of lines of code per function low.

The author doesn't understand the relationship, he blindly follows the advice to keep the number of lines low without considering the complexity and that leads to unnecessarily complex code.

Agree completely, it comes across that the author of the post has read some useful guidelines and created a strict rule which must not be broken.

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.

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.

Exactly. Here's a version of his refactored code which re-replaces the if statements with guard statements: https://gist.github.com/cellularmitosis/9a175facd5fe260403f1

Yes. And if all your code is composed of three-lines methods, then ok. Otherwise guard is extremely usefull, because unlike 'if' it leaves your constant/variable accessible outside the guard scope.

> If an article starts with basically "Because Uncle Bob said so", cum grano salis doesn't even cover my mindset

Uncle Bob is a recognised expert in the area. So no, his opinions on SRP should not just be dismissed out of hand.