In Ruby community, we have something like Rubocop to review and give feedback to us, developers, regarding code style. There are certain limits in which a method must contain less than the specified amount of lines.
You got 40 lines of codes in a method? It’s time to extract some of them to a new place. Even madam Sandi Metz has a rule that won’t let you have a method with 5 lines or longer, something like that. This way we can have automated pull request review, so when a PR violates the rubocop’s rule, such PR cannot be merged to master. And sometimes I can tweak the limit number as well.
Now, I’ve seen my coworker’s Elixir code which have a function with 112 lines of codes in it.
Somehow I want to enforce the limit, but I have no idea, how long it should be and which tool I can use.
I think the only thing that works reliably is manual code review. Tools that check function length tend to become an obstacle to work around without actually improving the code.
I agree that generally the number of lines should be short but there are use cases where it makes more sense to have longer functions than splitting it up into multiple pieces. In my experience a developer struggling with writing functional code generally will not make the right decisions to make a function shorter and need help to get into the right way of thinking.
Credo is a great tool, which performs many of the functionality you’d use Rubocop for in Ruby-land.
The nice thing about it, is that there are many style-related issues it hints at by looking at what you (and other contributors to the project!) do: If everywhere you use two spaces for indentation but here you use four, then it will warn you about it. But if you use four everywhere, and use two spaces at one place, then you’ll be warned about that line instead.
As for checking for function size: Credo checks at least for cyclomatic complexity (source), which is a matric that is slightly more sophisticated than lines-of-code for a function.
At least you may skip a rule with rubocop, if you want to.
But still, in my preference, limiting a subroutine with small amount of code is better. Small function with single purpose, small classes/modules, etc, make them easier to read, refactor, document and test.
Even though we do not do elixir in my company, there have been discussions about setting our linters to fixed body-lengths.
As we are using go and python as primary languages as well as puppet, docker and bash as secondary languages, we decided we can not agree on a limit that suites all of those languages, therefore we decided to not put a limit at all. But we do not merge without review. So if the reviewer is in doubt about the length, we need to discuss. But usually we refactor as soon as a single person is in doubt.
After about a year, we consider this superior over fixed linting rules, and after we learned to know each other better, we ran less and less in situations that did not pass review (because of function length).
A good starting point as a rule of thumb we discovered was: “Would it fit on Xs screen?” (X is actually me, when working from the park or coffee shop with my 14" laptop).
The author discusses how software design could be improved by increasing focus on how design decisions impact the system’s overall complexity.
You can watch a 1 hour talk the author gave about some of his ideas here. One thing he discusses is the idea that classes should be “deep” (see talk for definition), and in that context he touches on the “n lines rule” at 17:50.
Discussion of the book, and number of lines vs design is sprinkled throughout episode 20 of the Elixir Outlaws podcast, but is mentioned more specifically at 21:40.
To be clear: I’m just adding relevant sources to the discussion (I hope!), not taking sides
That’s why I think it’s an interesting part of the conversation
I don’t believe Ousterhout is against having (e.g.) small methods. However, he is saying that if you’re going to introduce a new method/class, make sure it will be reducing the overall complexity of the system or that change will be a net loss.
In other words, he advocates making things modular as a way to reduce complexity in our software designs, instead of as a goal in itself. One of his reasons to not blindly follow “functions can only have X lines”-type rules is that by doing so without critical thinking you’re just exchanging complexity within the method (assuming shorter methods are simpler to understand) with complexity in a higher level (because now you need to know a new function signature, a possibly how it is implemented). Kind of like what Rich Hickey means when talking about “guard rail programming” (about 16 mins in): adhering to these rules doesn’t absolve you of thinking about how best to design your software. I.e. your software can still be poorly designed even if you’ve religiously followed all “rules”.
Fundamentally, one of Ousterhout’s points is that if to understand what a function is doing you need to look at the implementation of the functions it calls, you may want to reevaluate splitting out those functions. Because if you need to know about the implementation of sub-functions to understand the main function, you’re not abstracting away anything at all: you’re just adding extra steps to understanding the code (via jumping around to sub-function definitions). In those cases, he says, it might be better to have (some of) those functions inline within the main function, so that everything you need to understand what’s happening is available where it needs to be understood (with the relevant context).
Honestly, I think that keeping functions small does help a lot with keeping complexity low. The problem is, splitting code across the right boundaries is not obvious, and choosing the wrong boundaries can actually make things worse. Also, good naming and good interface design is even more critical when writing many small functions. In sum, small functions are in my opinion a lot more readable (and testable, reusable, etc.), if who writes them is experienced enough, and has code readability in mind.
I would apply a rule such as “functions shorter than 20 lines”, but instead of enforcing strictly, require a valid reason when breaking it. A very long function (more than 20/30 lines?) really needs to have a good justification to exist, because in most cases it DOES mean that it’s doing a bit too much. Generally applying such a rule also is a good exercise for the team to learn to split, name, test, etc., provided that it’s not done mindlessly.
Rules are means to goals, so one always has to keep the end goal in mind: we keep functions small to keep the cognitive load in understanding our code low, and to make functions testable and reusable. If a code style tool helps a team keeping the attention high on code quality, then it’s definitely a good idea. If it leads to mindless enforcement of arbitrary rules while loosing focus on the goal, it’s damaging. It ultimately depends on the specific team and its culture.
