I can't think of a single benefit of a function over a comment for a piece of code that's only used once. The downsides of the function:
- Unless the function is extremely clear like max() or min(), you will have to read the function and jump around the code. I feel people underestimate the cognitive overhead required for this. This gets worse the more arguments are required to pass around. Linear code is much easier to read.
- More lines of code, a comment adds 1 line while a function or method adds at least 4. Also, code that has to be separated over multiple functions is practically always longer than linear code. Doing this to the extreme can have quite an effect.
- Naming issues, it can be hard to think of a good name for a function and it almost never describes it perfectly. Misnaming things leads to bad assumptions and leaky abstractions. Comments have the luxury of potentially conveying more information and are also less important to get right.
If you use a lot of vertical scrollbars then by all means go ahead and abstract it, but I'd generally pick the comment.
I think the most important drawback of the function for used once code is that its definition doesn't really map well to linear files.
There isn't really a proper place to put such function.
In this example, you would like to define add_vertical_scrollbar() right where it is used as this is the proper context where it makes sense to read it. But if you define it where it's used it no longer adds any clarity.
> I can’t think of a single benefit of a function over a comment for a piece of code that’s only used once.
A function makes the higher-level flow more clear and uncluttered than without either the function or comment, a comment may make it more clear but also makes it more cluttered.
> Unless the function is extremely clear like max() or min(), you will have to read the function and jump around the code. I feel people underestimate the cognitive overhead required for this. This gets worse the more arguments are required to pass around. Linear code is much easier to read.
Yes, function names should be extremely clear. But less code is easier to read than more code, so unless the contents (rather than the purpose) of the block of code is likely to be important every time you read the containing bit of code, you make the containing bit of code more readable by extracting the block.
> more lines of code, a comment adds 1 line while a function or method adds at least 4.
Depends on the language. In JS, for instance, a function definition can be a single line, so definition + call adds a minimum of 1 line, just like a comment. But a comment adds 1 line to the parent block of code, abstracting out a function at worst simplifies a single line, and usually reduces the lines of code in the parent block. So where the parent but not the content of the abstracted bit is important, it reduces code read, whereas the comment always increases it.
> Naming issues, it can be hard to think of a good name for a function and it almost never describes it perfectly.
Whether you can think of a clear name for the function is, IMO, part of the test for whether it is a logically coherent unit of work to abstract out to a function in the first place.
> Comments have the luxury of potentially conveying more information and are also less important to get right.
Comments are a pure addition of clutter to the code they are attached to; if they aren’t gotten right they are not just visual noise but misleading visual noise. Comments are no less important to get right than naming of functions.
> A function makes the higher-level flow more clear and uncluttered
It is subjective and depends on a given piece of code. In my experience functions (abstraction in general) often obfuscate code, because you cannot really see what they do without jumping to another place and breaking a flow of reading.
There are of course cases where factoring a piece of code into a function would make in more readable, but I cannot say this about any group of commented lines. And a function requires a comment too.
> In my experience functions (abstraction in general) often obfuscate code, because you cannot really see what they do without jumping to another place and breaking a flow of reading.
Already discussed in the post you are responding to: “unless the contents (rather than the purpose) of the block of code is likely to be important every time you read the containing bit of code, you make the containing bit of code more readable by extracting the block.”
> There are of course cases where factoring a piece of code into a function would make in more readable, but I cannot say this about any group of commented lines.
Also already addressed, e.g.: “Whether you can think of a clear name for the function is, IMO, part of the test for whether it is a logically coherent unit of work to abstract out to a function in the first place.”
> And a function requires a comment too.
Not necessarily. A function in the public interface of a module probably needs a doc comment or docstring, but we’re discussinf abstracting out a piece of functionality used once, so presumably this is to a private function in the same module, not part of the public interface of the module.
What border layout does that function use? Hardcoded BorderLayout.EAST, as in the code it replaces? Now the function is much more specific than the name suggests. We have a function that looks like it could be used in other parts of the code too, yet was only intended to replace one specific piece of code in one specific function. This is my main problem with extracting even small pieces of code into separate functions, especially in languages that don't have nested functions or similar facilities.
When reading the calling function, add_vertical_scrollbar() makes perfect sense. But when you read the code and you encounter add_vertical_scrollbar(), it is not at all clear where it fits in the grand scheme of things. You could of course add a comment "Meant to be called from this-or-that-function", but that kinda defeats the purpose.
We could make the function more general, which is not always as simple as in this case, but then we're doing more than just extracting some code. Or we could call the function add_vertical_scrollbar_east() instead, but that gets unwieldy pretty fast if there are more tunables.
Don't get me wrong, I do see value in extracting code into functions, and I often do exactly that. But in doing so I notice some drawbacks of that approach that IMO are not generally addressed by small-function-advocates.
In fact, coding in paragraphs and factoring chunks of code into separate functions are not mutually exclusive or contradictory; coding in paragraphs facilitates such factoring.
It's very easy to generate code that "jumbles" tasks. You set "x" coordinate of your point in one place, then do some calculations for "y" coordinate, then set it, then do something unrelated, and then set the color of your point. Such code does not make it obvious that you could create a "set_point(y, y, color)" function.
When coding in paragraphs, you organize your code into chunks so that each one "does one thing and does it well". It's natural then to consider whether a particular chunk could be a separate function.
Often it is, but sometimes it is not. First, creating a separate function has costs: choosing an appropriate name, designing an interface, etc. If the name and/or interface are not well-designed, a separate function can decrease readability: consider a function called "fix" that has 10 positional arguments...
Second, especially if your chunk is within a loop or two, the code may be too tightly coupled with the state within your code. You would need additional and perhaps modifiable "state" parameters to your function, making the resulting code more complicated and less comprehensible than the original one.
And of course, if your hypothetical function cannot be inlined (perhaps because your language does not even have the facility, or for other reasons), you would pay the price of a function call, which may matter in a tight loop.
In general it's always good to consider factoring chunks of code as a separate function, but sometimes upon this consideration you should reject it.
In my opinion there are two approaches to the idea of a function
Some use it as a method to separate code into independent, concise blocks that represent a singe step in a larger algorithm. Do one thing and do it well.
The other is code reuse, if a function is not reused in other places why create it at all? That's the reason for preferring comments rather than fragmenting code into add_vertical_scrollbar() functions. It's easier to read from top to bottom instead of jumping all over the place
It triggers half of HN because taken at extreme (abusing) this advice will render code unreadable again.
It is never clear cut. But if you never reuse the same code, or it is targeting the wrong abstraction layer (you can't reuse the function without introducing another set of arguments) and you are merely encapsulating two already very readable lines of code, you might end up with a too much redirection and an increase in mental load.
What I'm trying to say is: It might be even harder for the outsider to follow/read the code, just simpler on first glance.
If the lines of code were already very readable, they wouldn't require a comment. Comments are for interfaces or complex or inobvious code that can't easily be made more obvious simply through an expression of code IMO.
The code example of the OP is excellent because there are literally so many more avenues for code readability that are all superior to comments. First of all, the same code can literally be reused in the next line:
addScrollbar(HORIZONTAL);
addScrollbar(VERTICAL);
Do you frequently or always need to add both scrollbars to the UI? Another chance for reducing repetition and error-proneness when refactoring:
addScrollbars();
There's no increase in mental load because chances are you'll never need to navigate into these functions to know what they are doing at all, you can read over them as you would normally when you read the "This adds a horizontal scrollbar" comment, and trust that the implementation does what it says on the tin.
It seems you did not try to understand my comment:
I said "taken to the extreme". Put differently: If you are going to wrap every pair of lines into separate functions as a way to structure your code, you code will end up becoming functions calling functions calling functions - and whenever you try to read the code, you are required to jump through all these redirections.
Now instead of a bunch of prepared and labeled dishes you have an API border in the middle of your kitchen, which of course will greatly help at slight changes in the menu. When in doubt, just pepper it with more enums and boolean flags. /s
add_vertical_scrollbar()