How much information is too much information? (2022)
sourcery.ai
sourcery.ai
The first function, on the other hand, has only static assignments (one name is only ever bound to a single "definition" at any one time). In terms of working memory, that means that each variable in the first function only occupies a single "slot". In the second function, the mutable `min_value` variable necessarily occupies multiple "slots", one for each point in the program where the variable could change. So you'd have to keep track of a "min_value[0] = 9999", a "min_value[1] = number (if number odd && < min_value)" and _maybe_ a "min_value[2] = min_value (if number even || > min_value)".
For the same reason I found that their refactoring actually made the first function worse. Now I have even more jumps that I need to make in order to 'inline' all the code so that I can deeply understand what the function is doing.
There seems to be an implicit assumption that you should just trust the names of the functions and not look at their implementation. This is hopelessly naive. In the real world, functions don't always have obvious names, their implementation can involve subtleties that 'leak' into usage etc.
I strongly dislike function extraction that's driven by anything except the need to reuse the code block. Function extraction for readability is about as useful as leaving a comment above the code block. Honestly I'm more likely to update the comment than the function name since changing the fn name means updating the callers as well.
edit. To add a bit of nuance: I think 'primitive' vocabulary is what's essential here. The standard lib of a language is primitives. The standard functions of a framework like Ruby on Rails are primitive.
I'm happy to see code written by calling functions/types/operations that I have seen before dozens or hundreds of times. What I don't like is when a feature in a codebase has created it's own deep stack of function calls composed out of functions that I know nothing about.
Create a rich base vocabulary that is as widely shared as possible and use that for your work. Avoid creating your own words as much as possible. This way I can glance at your code and not just see if (BLACK_BOX_1 or WHAT_DO_I_DO_AM_I_LYING) then DO_SOMETHING_BUT_MAYBE_I_ALSO_DO_SOMETHING_ELSE_WHO_KNOWS
If I'm only reading that code like ... 1 time ever, I'm never going to add to memory all the dozens of random 2-line private functions someone broke up their 100 line procedural function into. I'm going to be annoyed and mentally 'inline' it all into one linear block of code and then forget all about it.
Therefore I'll never remember or trust the private function names and will never know what the module does at a glance. I would be better off with it just inlined into one big procedure in the first place. Don't pollute my mental function cache.
The more some code gets touched and read and updated the more I can justify adding an internal vocabulary to it. Again, something like Ruby on Rails has a huge API surface area compared to plain Ruby, but that's fine, because after using it for a decade I know most of it and it was worth learning the vocabulary since I'm constantly using it.
It's like... commonly reused library or std lib or framework code is useful jargon used and reused by a community of people. But that pattern doesn't downscale. Applying it and creating my own jargon for an obscure module in a code base is a bit like someone inventing slang all on their own and whining when people ask them to explain what they mean in plain english. I don't want to read Finnegan's Wake for my job, just give me some boring realist prose suitable for a 5th grade reading level.
In such a case the probability a function does something other than what it says on the tin is almost 0. If I am using Forth, the probability of WHAT_DO_I_DO_AM_I_LYING doing something naughty is much higher. In Haskell, it's already pretty unlikely.
Maybe preferences in this regard depend on language and peer trust?
Whether the code is split into 100 little helper functions, or is just one long block of the same code inlined into a single function, or somewhere in between - those are view issues. They do not have - or at least should not have - any implication on semantics. It makes zero sense to try and commit to some one perfect balance, because at any given moment, I may want to have everything inlined, or everything extracted to functions, or everything inlined down to two levels deep in the call stack, etc. It all depends on the reason I'm looking at that piece of code in the first place.
This is one of those holy wars that can't be won, because the problem is the limitation of the medium. Similarly with increasingly arcane ways of lazily flatmapping monoids across sets of endofunctors, all to avoid "callback hell", hidden state, non-local control flow, incomplete type definitions, etc. - all at the same time. You just can't do that - the reason bleeding-edge PLs get increasingly weird with syntax and require math degree to grok ideas behind them, is because they aren't improving things - they're just sliding along the Pareto frontier[0], hitting the boundary of plaintext capacity. There's only so many cross-cutting concerns you can express simultaneously in the same piece of text, without making it impossible to understand. And the irony about cross-cutting concerns is, at any given time you want to ignore most of them.
What I mean is, say I'm reading your 100-helper-methods "clean code" function, trying to fix a tricky bug in the algorithm it's implementing. The helper methods are an annoyance, I want them all inlined (with properly interpolated parameter names - i.e. the exact inverse of "extract method"). Also, unless I have a reason to suspect the bug is related to error handling, I don't want to read any of your fancy Result<T, E> noise you use in lieu of exception handling. Hell, I don't want to see exceptions either - I want all error handling logic to disappear entirely (or be replaced with a bomb emoji, so I remember it's there). Same with logging/instrumentation, and a bunch of async calls and the whole "color of your function" bullshit. Not relevant to my problem, I don't care, I don't want to see this. Hell, I probably don't care about most types involved either.
Next week, I'm back to the same code, trying to improve some logging and fix the case where error information isn't propagated. Suddenly, I now want to have those 100 helper functions be their own named things. I want to see Result<T, E> - in fact, I probably don't want to see the T part. Logging? Yes. Async? Probably still no.
A month from now, the impossible happens, and I'm given time to optimize performance of that same piece of code. Obviously, this calls for different set of readability tradeoffs (and more than anything, the ability to change them in flight).
So how about we stop wasting all our combined brainpower on arguing in circles over fake problems, which exist only because we insist on only ever working with the one, single, canonical plaintext representation of a program? It's literally the software equivalent of one-size-fits-all shoes - no matter which size you pick, the fit will be bad for almost everyone. Exceptions vs. Result<T, E>, or "lots of small functions" vs. "few big functions", etc. are all just different ways of asking which shoe size is the bestest shoe size - the problem isn't the shoe size, but that we have to pick a single size for everybody.
--
But maybe it would be easier in some languages than others. Lisp, being essentially a naked syntax tree, might be amenable to this. And Go has automated formatting tools. Granted, they aren't used for this, but perhaps they could be extended to do so.
But doing this for something like C++? That seems like something that's going to take a long time.
For an incremental approach, we could treat C++, or Python, or Lisp, as they are today, as serializing formats. We can keep using the existing process of compiling source code into executables - what matters is that we stop writing that code directly. That doesn't mean write no code at all, but rather breaking up with the idea that what we write the exact text files that then get compiled into programs.
Instead, the way forward is to treat the source code as more of a database, an artifact we sculpt (and yes, eventually it would be beneficial to ditch the plaintext source file representation too). This means the same underlying source code can be viewed and edited in many ways, textual and graphical, tailored to help with specific concerns or abstraction levels. So basically, take the modern Java/C# IDE, but instead of treating its advanced functionality as something to save you some typing, embrace them as the main way of looking at and modifying code. Then take it up to 11.
There's a precedent to that, too. It's not Lisp though - it's Smalltalk.
I'll postulate a core goal, with regards specifically to optimal abstraction/inlining, involves ensuring the functional dependencies mirrors the readers internal conceptual representation of the problem domain as closely as possible. I consider this property as part of legibility and more important than readability. What it sounds like you are saying is that different tasks involve different conceptual representations and thus need different views, and I find that reasonable.
I'm not convinced that this is possible right now - if at all, however. The crux of the problem is that extraction supporting more than the abstractions you already implemented would encounter obstacles. I suspect automatically abstracting would be a hard problem. And the function naming problem would re-emerge - both you could give functions wrong names, and also your system could. In some cases there will not even be such a thing as an acceptable name, unless you assign random strings, in which case you'll have to check the abstracted code anyway to see what it does. And type signatures/property-based tests can't save you here, because the system would have to determine those. So automatically acquiring different views of the code may not be feasible.
Maintaining multiple views of manually implemented function hierarchies over the same data also sounds even more prone to name/doc drift than conventional formulations.
My point here is that for extraction to work, you have to implement the code you are working on in terms of all the helper functions already and you'd still have to deal with how you chose abstractions and names and description drift and so on.
Your concrete suggestions seem to be to use an IDE that can inline and fold code, use a binary file format for code, use auto-formatting tools, and give your language multiple ways to do each thing so you can have the IDE reformat a block of code with a different style to change how is presented. The only new feature may be using a ML model to tag code according to purpose so you can hide code by tag or hide all code not matching a tag.
While I agree those could be useful, we'd still end up with a canonical lexical function dependency hierarchy to obsess over. The problem of inlining vs abstracting still comes up there. Now abandoning that would be something, but I don't know how to do it.
I think the debate still exists because editors/langs don't work like this and you end up having to compromise one way or another. Obviously, if we could do really fancy code folding / inlining / condensing based on arbitrary user defined filters everyone would just write whatever they wanted.
I guess code review might be the only place where people would still complain, depending on how that looked.
I do agree with the rest of the article. It's the same stuff I've been doing since reading Clean Code over a decade ago.
This gets me thinking, there really is an opportunity cost associated with spending a lot of time working with sequence abstractions and immutable data structures. The better I get at reading immutable code, the harder it is to read code with mutation.
With the highly unusual addition that there's an extra if statement inside. That's weird.
And the initial value of 9999 is highly suspicious. All the numbers may be higher than that.
I'd say it has complexity value 4 -- "find the minimum value, in the usual way, except only look at odd numbers, and use 9999 as the initial value".
That you know well. This is why there's so much disagreement on this stuff. Different people have different idioms that they know well. You're never going to be able to reduce "what is simple" to something everyone agrees on.
There's an actionable part of this observation. If you're responsible for a code base, it should use a limited number of common idioms. Have everyone working on the code base learn and use those idioms in preference to other approaches. Consistency reduces the mental burden on the reader.
This is an unappreciated value of using code frameworks and doing things the 'framework' way. You might have a somewhat convoluted function using framework or std lib functions to do a task but anyone familiar with the tool can just read it top to bottom and have a genuinely deep understanding of what the code those and what edge cases might show up.
If you convert that into your own function vocabulary by wrapping the 'basic' code into extra methods... nobody can read the superficially more elegant function and know anything more than what the function names are telling them. It's like they're reading pseudo-code and can only guess at the implementation subtleties unless they annoyingly 'manually' inline the code by jumping to and reading through the custom function definitions.
I don't know, does that mean it's declarative? I don't understand how that's supposed to be more difficult than stepping through loops.
Our profession seems to consistently underestimate or even ignore the complexity of interactions between many little pieces.
I would rather work on a module with 10 clearly-written 500-line functions than one with 500 clearly-written 10-line functions. Keeping in my mind the complex and variable interactions of a whole constellation of tiny functions is much harder than grinding my way through understanding a few more complex pieces of code. Have you ever worked in a codebase where the template method pattern was combined with too many levels of inheritance and each subsequent method is implemented at a different level of the hierarchy?
Likewise for microservices and lambdas. The organizational complexity of an app atomized into 100 lambdas is mind-boggling, because you have to understand the implicit and emergent orchestration of distributed state as those lambdas transition from one to another.
All that said, I definitely like this metric better than most others for the static complexity of a single function.
Maybe, maybe not. The real question is, how clean are the interfaces? How good are the functions at being abstractions? Write your functions to provide a clean interface to some functionality, not to be a set size.
If the interfaces are clean, then it's better to have 10-line functions rather than 500-line ones. Why? Because I can be certain that this 10-line chunk doesn't affect the next 10-line chunk, except in these few, carefully-defined ways. In a 500-line function, I don't have that guarantee. I have to worry about how something 400 lines away affects this line. If I've broken that 500-line function up into 50 10-line function calls with clean interfaces, I can see whether that thing can affect this thing.
But only break it up if you can do it cleanly. There's no point sawing a 500-line function up into 50 10-line functions just to have shorter functions. If the function doesn't decompose cleanly, you wind up with interfaces that are a mess, and you wind up with more complexity rather than less, because you added the complexity of the interfaces (and the indirection) to the existing hairball.
1) Stack depth. In their refactored example if I truly want to understand the code I need to now store everything I was thinking about in the current function and jump to their new `is_paid_today` function, then into each of the 3 dependent functions, I'd say as finger in the air type estimates each stack frame should add 2 * depth and then some multiplier for breadth or something. This is why I prefer big functions like the initial state read from top to bottom with no indirection, abstraction or surprises where reasonable. (curly braces wouldn't go amiss to give visual indicators of scope...) This is completely counter to 'clean code' dogma but clean code dogma is... bad.
2) Type tracking. To reignite this holy war if I have to track the type information mentally then I'd say every parameter and variable has an additional +1 working memory overhead. What is employee, what is employee_database, what is `is_end_of_month` and where on earth is it coming from? Try as I might I just cannot understand people who onboard to big codebases without type hints at least, y'all are a different breed, you should work as like super-rememberers.
Type tracking though, oh yes. How many times have I been debugging old JS code where someone forgot or misunderstood that this date is a string and not a Date.
I'm usually reading code for 2 purposes. First, to understand the code and the domain. If this is the case I want to see how the code handles paid weekly versus monthly. Is it possible it's not accounting for fortnightly (biweekly) payments? If we were to account for that how would it slot in. How does `is_end_of_month` or week account for federal holidays, etc?
Secondly, to fix it. Something has gone wrong or some assumption has become outdated. The call is coming from inside the house. Nothing is to be trusted. This is most of my work to be honest, spelunking into code and trying to solve the mystery in which case I'm not going to take the current name-as-written to be factual and need to include the function call in my logical model of the code under investigation.
That's not to say I disagree entirely. Like all things in software it's a balance and you don't want to define getting a lighter color everywhere you need to use it, eventually you want to have a function that just wraps it. But over-eager splitting out of functions also incurs a working memory cost and it's striking the balance between the stack depth and memory overhead of the single function. Since the article's working memory model incurs no cost per stack frame I don't think it's well designed to encourage careful thought on this trade-off.
The thing people don't often get is that when I code it's to a purpose other than making the code. So I'm thinking about quite a few things outside of the code that's making those things happen with the samples I'm working with, and the stuff not related to how it's coded is typically a lot more important.
Hence, my stack is always going to be catastrophically, tragically pitiful, unless the thing I'm working on is quite useless :) and in that case, why am I bothering to code at all?
I recently had a developer "improve" a security sensitive series of checks (think a series of if statements: if logged in, if not inactive, if in required group, etc.) by merging the statements into a single if statement that, unsurprisingly, broke the check and left a security hole.
Of course, there's also a gap in unit tests if they can change an important check without a test failing.
So, 50% success, I guess.
Belt and suspenders.
bool hasAccess = HasAccess(token);
private bool HasAccess = token: Jwt => { return IsLoggedIn(token) && IsActive(token) && HasRbacPermission(token); }
If so, what's wrong with that? It moves the complexity of HasAccess out of whatever is checking for access, before presumably doing other things.
* I have each check on its own line, don't want to reformat message to take up space here.
if(!loggedIn) return Unauthorized;
if(isTokenExpired) return Unauthorized;
etc? I think being that explicit about it works pretty well sometimes.
Check out some of the other comments on here: It's bad advice because they don't account for a whole bunch of other things.
For example, "volume = width * length * height" is less demanding than, say, "employee_qualifies_for_bonus_after_midterm(emp)". I think it comes down to limiting the spatial scope of variables and fragments of code, naming things in a way that makes sense to most people, and containing complexity locally. It's also called "abstraction".
There's a dogma that variable names should be super descriptive, but I came to programming from science where X, y, phi, gamma can be both highly generic and ambiguous and highly meaningful (everyone knows what X@beta means). I try to find those opportunities elsewhere.
Or at very least start a function that takes an `employee_qualifies_for_a_bonus` argument and immediately say `eqlf_bonus = employee_qualifies_for_a_bonus`. There's a time for understanding the domain and a time for understanding code. I
This crystallized for me why you shouldn't interrupt devs when they are working and keep interruptions to a minimum.
Briefly, the author asserts, that writing software is like sleeping. For a lot of people getting to sleep takes a while and if you wake them up even for a minute, they have to go through the 'getting to sleep process' again and it's non trivial.
Software complexity is the same, it takes a while to go through the cognitive exercise of grasping what the code does. If you interrupt me, I lose the context and have to go through the "code loading" steps again..
I tried to explain this to my wife, unfortunately she's a rare breed that can fall asleep on a dime so this explanation did not make quite the impact that I was hoping for.
if (
(
has_passed_probation(employee)
and is_paid_monthly(employee)
and is_end_of_month
) or (
is_paid_weekly(employee)
and is_end_of_week
)
):
The smallest odd of a list of numbers > 9999 is not 9999.
The smallest odd of an empty list is 9999 is not 9999.
The smallest odd of a list of even numbers is not 9999.
The first function does seem easier to understand but relies on global state (is_end_of_week, is_end_of_month) and side effecting functions (run_payroll).
Neither of these functions should survive code review.
Given that there would probably be more complexity in the real code I would create separate functions expressing the rules for each employee type.
employees
.filter(has_passed_probation)
.filter(...)
.forEach(p -> run_payroll(p, p.salary))
The only issue is that it's a bit annoying with breakpoints sometimes.
"just" in this case means as of March 2014.
* The table-like structure of the conditions instead of nesting like the second one. The two-dimensional relationship makes it trivial to reason about - understanding it works at a higher level than the individual variables.
* The body of the conditional is entirely self-contained and can be understood separately from the conditional instead of having to understand them both at the same time.
The refactor they suggest at the end only works so simply because of my second reason here. It's kind of an improvement (self-contained logic that can be reused), but kind of not (now you have to look in two locations to actually understand what it's doing), but either way isn't really necessary if you read code at a higher level than token-by-token.
The second function is harder to to understand in the sense of meaning, but easier to understand in the sense of raw behavior. I can see more of the code that determines it's actual behavior, and can inspect and trust that the logic is appropriate.
In that sense, if we accept that `<` and `%` have sensible definitions in the second example, it would only be fair to assume the same for the helper functions first example.
The first example did have a bit more complex condition, but it was fairly logical and not hard to understand. Again what bothered me was that there must have been a hidden way for the result of run_payroll(name, employee.salary) to pass its result to write_letter(name)
Must there have been though? Sounds like you’ve made an assumption about the contents of the letter. It’s probably as simple as “John, you were included in today’s payroll run.”
On the other hand, in this style the first program could have any amount of hidden traps.
Ouch, and this immediately makes it less obvious what's going on, because you have to look up the function. In this case, the name is self-documenting, but that's far from always the case. And even then the extracted function seems to conflate 2 concepts, 'pay day', and 'eligible to be paid', and the code even looks suspect/buggy; which is more clearly unambiguous:
(one and two or three)
(one and (two or three))
> the number of distinct pieces of program state that a developer needs to keep in their head when analysing a function and the impact this has on the understandability and maintainability of code.
The same concept applies not just to writing code, but to any context that you need to load up in order to do your work. E.g. context when jumping into a meeting or when reviewing a new project spec.
In those cases, one of the key things is being able to offload what you don't need in your working memory right now to some other solution (usually notes). More on that here https://www.stashpad.com/blog/working-memory
What? It’s one conditional branch, and in that branch there are three “high-level” operations, all named very well (assuming they’re correct names.) What’s harder to understand here?
I spent much longer understanding what was going on in the second function.
Further, the variable name “paid_today” is terrible because the result of the condition does not tell us that the employee was indeed paid, but rather that they should be paid. They’re not paid until after the code in the conditional branch runs.
Extracting this information to its own function is certainly an improvement (even if the function name still isn’t great.)
I would have liked the article to spend more time making the second function take less time to understand.