Doesn't matter how it looks. If its not possible to understand what a function accomplishes within a reasonable amount of time (without requiring hours upon hours of development experience), it's simply bad.
So, I'd take "good architecture" with ok and above readability, over excellent readability but "poor architecture" any day. Where architecture in this context means the broader code structure of the whole project.
The novel function that might take "5 seconds to read" for the 20 people contributing to a mature project with a good architecture might nonetheless take 10 minutes for a new hire to decipher because they don't know the local vocabulary (architecture, idioms) and need to trace and interpret things elsewhere in the project and its libraries.
Meanwhile, writing implementations in a way that tried to avoid a local vocabulary altogether might make naive reads easier, but less readable to experienced team member because they're not as concise as they could be.
Your general advice to "make things easily readable" is good advice, but like with writing compelling prose or making accessible graphic design, you need to consider your audience and your choices might look different than the ones somebody else might make.
This is where “simple recursive data structures” can be simple to read but difficult to track and comprehend. An architecture that descends through distinct layers at least has landmarks that the recursive one does not have. If you are not at the root or the leaf you don’t really know where you are, lost in the middle.
function isReadyToDoThing(Foo foo) {
return foo.ready
}
function processStuff((Foo foo) {
if isReadyToDoThing(foo) {
res = workflowA(foo)
res2 = workflowB(foo)
return res && res2
}
}
But also consider:
function isReadyToDoThing(Foo foo) {
return foo.attribute1 && foo.attribute2 && ! otherThing(foo)
}
Making these kind of tradeoffs involve thinking about the overall system design, not just the way you structure the code within a given function.
I think people get too caught up in "small functions" and lose the readability of code locality.
Further - the more of them you need (call depth) the worse this problem becomes.
But who is the viewer of that name?
How much context can they be assumed to have? The name of the class? The name of of the module? The nature and terminology of the business that the function serves? The nature and terminology of the related subsystems, infrastructure and libraries?
There is a context dependent local optimum for how to name something. There are conflicts of interest and trade-offs have to made.
All of this.
That's a good rule for straightforward CRUD apps and single-purpose backend systems, but as a universal declaration, "it is simply bad" is an ex cathedra metaphysical claim from someone who has mistaken their home village for the entirety of the universe.
I have a cargo ship-sized suspicion that your code is difficult to read for reasons other than intrinsic complexity.
You’ve found a way to explain it to yourself and excuse it to others, but you won’t always be the smartest person in the room.
Also that’s not what was said.
> more then 5 seconds to read and understand the high level goal of a function
Understanding what something is for is not understanding how it accomplishes it.
You sound unhappy and seem to be lashing out, and your username can only be read as an allusion to a mentally ill would-be assassin. Given those, you can maybe begin to understand why your opinions are not credible as a contribution even in relation to other anonymous people.
A small part of your comment is salvageable, though:
> Understanding what something is for is not understanding how it accomplishes it
I can think of at least one area I know something about where the 5 second rule fails - sometimes when working on a shader and optimizations for it, it takes more than 5 seconds for the person who wrote the code to describe what it's for at a high level.
If even the person who wrote the code can't meet that arbitrary constraint, other people looking at the code for the first time have no chance.
That's something that's possible only for fairly trivial logic, though. Real code needs to be built on an internal "language" reflecting its invariants and data model and that's not something you can see with a microscope.
IMHO obsessive attention to microscope qualities (endless style nitpicking in code review, demands to run clang-format or whatever the tool du jour is on all submissions, style guides that limit function length, etc...) hurts and doesn't help. Good code, as the grandparent points out, is a heuristic quality and not one well-defined by rules like yours.
I meant the goal of your function needs to be grasped within a reasonable amount of time. This works for every codebase.
It really doesn't though. Here's a function of mine. It's maybe 40 lines of logic, so medium-scale. It's part of an intrusive red/black tree implementation for Zephyr. I'm fairly proud of how it turned out, and think that this code is awfully readable given its constraints.
No human being is going to understand fix_extra_red() without having already read and understood the rest of the file, and coming to it with an understanding of the underlying algorithm. Certainly I can't. I can't even get started on maintaining this code that I originally wrote within a five minute time frame, it's an hour at least every time, just to remind myself how it works:
https://github.com/zephyrproject-rtos/zephyr/blob/main/lib/u...
Now maybe this is "bad code", and "good code" could exist for this problem that still meets your requirements. But... if so that's an awfully celestial definition if it's so hard to find.
Maybe it's a defeatist attitude, but I feel like sometimes the problem is the problem, and pushing abstractions only works to defer the requirements to understand it. Sometimes you can defer it enough to do useful work, other times you just need to understand the thing.
That's also my impression and experience.
And sometimes there is no problem at all, but abstractions are still pushed too far and then a problem arises in the form of non-essential complexity.
I suggest that a codebase should read like a newspaper. While there is room for op-eds in the paper, it's not all op-eds, everything else should read as a single voice.
My experience is that projects which value code formatters (and similar rulemaking) tend strongly not to have room for "op-eds", FWIW. And conversely the code bases I've seen with the best/cleanest/most-clearly-expressive code tend strongly to be the ones with fewer rules.
I think the one exception there is in some open source contexts (Linux is the archetype) which receive a fire hose of submissions of questionable quality and maintainership. There, you can use adherence to arbitrary rules as a proxy measurement[1] for attention and effort on the part of the submitter. And that I have no problem with.
But the actual value of code formatters is IMHO extremely low in practice, and the cost isn't "high", but is non-trivial.
[1] The "No Brown M&M's" trick.
Consider reading kernel or driver code. These areas have a huge amount of prerequisite knowledge that - I argue - makes it OK to violate the “understand at a glance” rule of thumb.
For example if you are calling functions "openDishwasher", "loadDishwasher", "closeDishwasher", "startDishwasher", your function should be called "washDishes". Not always that straightforward, but I believe in 95% it's not difficult to put a name on that. For the rest 5% you need to get creative, or maybe you realize that you didn't group the function calls well enough to have an atomic meaning.
I have no idea what that’s about, but I think it has something to do with “white-knuckling”.
People name things and then miss boundary conditions that matter and would have been implied by finding a more accurate synonym. And also supplementary features that the better name suggests.
I could see how that might come up in a retrospective.
I feel like there's a fundamental difference in the information density between code that, for example, defines some kind of data structure (introducing a new 'shape' of data into an application) versus code that implements a known algorithm that might appear short in line length but carries a lot of information and therefore complexity.
But does using a function, essentially a box with known inputs and outputs, constitute actually understanding the function? What happens if you need to debug or understand the implementation of it? Now the original name has gone and you're looking at a larger number of differently-named things that hopefully communicate their intent well. But if you need to understand _those_, and so on...
Doom famously has a function in it so obscure that nobody remembers how they even came up with it.
That's the mindset that the author is trying to counter.
https://learn.microsoft.com/en-us/dotnet/csharp/language-ref...
While Five Whys works very well for disaster prevention, I find 3 often suffice for fixing rather than explaining an architectural wart. Often we used to need this to work this way because something else had to work a particular way, but as the product grew that is no longer true, or desirable. So you might be able to fix it or put a fix on the backlog.