7 ms·

How much information is too much information? (2022)

89 points by gus_leonel 3y ago

vander_elst 3y ago
Are there any implementations of this? So that it's possible to measure it for a given piece of code?
Ensorceled 3y ago
I have a hard time getting developers to follow this type of advice and a lot of developers will "clean up" and "optimize" the example code in introducing_variable_version by eliminating the paid_today variable back into a single if statement. 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.
- Cthulhu_ 3y ago
  Yeah that's a constant struggle, developers thinking less and / or clever code is better code; it's self-gratification, and it should be stopped during code review. To them I'd say I'm sorry a series of simple if- statements doesn't challenge you intellectually, but the correctness of these checks are more important than your ego. Of course, there's also a gap in unit tests if they can change an important check without a test failing.
  - Ensorceled 3y ago
    It was found during code review AND it revealed a gap in our unit tests. So, 50% success, I guess. Belt and suspenders.
- throw1234651234 3y ago
  What was bad about it? Did they just do: 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.
  - Ensorceled 3y ago
    It was fairly long series of checks, some complicated, and they mixed up an or and not case when merging them into the single if.
    - throw1234651234 3y ago
      Ah, I see now. I think it's a very common mistake. People also mess up negation when they refactor things like these. So your original was if(!loggedIn) return Unauthorized; if(isTokenExpired) return Unauthorized; etc? I think being that explicit about it works pretty well sometimes.
- Izkata 3y ago
  > I have a hard time getting developers to follow this type of advice 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.
fit2rule 3y ago
[dead]
TexanFeller 3y ago
Having low working memory combined with my working memory slots being overwritten my random digressions my brain decides to pursue, AKA ADHD, reducing the amount of state I need to track is one of the most significant factors for productive coding. I guess that's why I find FP style code that eschews mutable variables and collections and anything that breaks referential transparency(AKA the ability to understand code locally, without surrounding context) appealing.
klauserc 3y ago
Interestingly, I had the exact opposite reaction. I found the second function harder to reason about because it heavily relies on mutation and the fact that individual loop iterations commute with respect to one another. (To understand what the final value of `min_value` is, you first have to understand that `<` being transitive means that the iteration order doesn't matter) 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)".
- JackMorgan 3y ago
  I agree, I read the first sample almost instantly, and I'm only _pretty sure_ I follow the second one after staring at it for a minute. Changing it to a range+filter+fold would make it an order of magnitude easier to read for me. 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.
- Scarblac 3y ago
  It depends on how often you have written loops just like that though. To my eye, setting a min_value, then looping through the numbers updating min_value when something lower is found and returning the end result is just a single "chunk" that's immediately obvious. 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".
  - deleted 3y ago
quantum_state 3y ago
is there a tool to scan the code and output the working memory metrics for a given cognitive context such as line, etc.?
drittich 3y ago
I'm still left wondering whether the code is correct, in that an employee that is paid weekly does not require passing probation to get paid. Is that the correct logic, or is it the (very common) error of forgetting an extra set of parentheses when using OR? Given that there would probably be more complexity in the real code I would create separate functions expressing the rules for each employee type.
- xmcqdpt2 3y ago
  Exactly. In a better programming language (like Java!) you could simply use sequential filter steps to avoid all those issues, employees .filter(has_passed_probation) .filter(...) .forEach(p -> run_payroll(p, p.salary)) In general I find those kind of examples useless because the real life version tend to be a lot more complex with dependencies all over the place. All the functions in the article are simple by comparison. In general I don't think it's always better to have six small functions split over four files that are required to do X instead of one 40 line function that has all the logic together.
  - throw1234651234 3y ago
    Have fun troubleshooting that. Also, iirc Java just got this in Java 8. Far behind .NET, as usual ;)
    - rufus_foreman 3y ago
      >> Java just got this in Java 8 "just" in this case means as of March 2014.
    - klauserc 3y ago
      I worked on a big Java project where we had to do a lot of list/collection wrangling like that. I found that code using combinator chain/list comprehensions/Streams/LINQ/etc. was significantly less error prone than code with hand-written loops. And that was in a team mostly staffed with junior engineers :)
UglyToad 3y ago
It feels like their calculation missed 2 crucial confounders, stack depth and type tracking. 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.
- jagged-chisel 3y ago
  Stack depth isn’t an issue for me typically. I assume the function is named correctly and the operation it represents is the operation needed at this point. If I find errors (why did we “getLighterColor” and from the resulting object expect to find a fruit name?), then I investigate the called function to see if it really does stuff a banana into a color and ask its author “wat?” And that’s because the contents of a function the current code depends on should be correct based on its name. 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.
  - UglyToad 3y ago
    I think while the name things correctly principle is nice and good to aspire to like anything at the intersection of people doing the right thing and natural language there's a bunch of places it doesn't work. 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.
zellyn 3y ago
I agree with all of this, except the part where they assume extracting a function is essentially free. 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.
- thanatropism 3y ago
  I mean, since we split systems up because the whole can end up being worth more than the sum of its parts, there's definitely a knowledge of the whole to be kept besides knowledge of the parts. You have to budget for this.
- AnimalMuppet 3y ago
  > 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. 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.
bluetomcat 3y ago
The "rules" are overly defined with respect to the static elements of source code. Cognitive load in understanding code is not linearly tied to the number of variables, function calls, conditional checks, etc. 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".
- thanatropism 3y ago
  Isn't variable name length a factor there? 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
heikkilevanto 3y ago
I believe a lot of this is affected about how familiar idioms are used. The second example simplifies to me as something like "Loop to find the smallest", "check parity". The thing that seemed to take most of my cognitive capacity was the unstated assumption that 9999 was clearly bigger than any of the possible numbers. 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)
- jagged-chisel 3y ago
  > … there must have been a hidden way … 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.”
jagged-chisel 3y ago
> the top function is fairly obviously harder to understand 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.
cborenstein 3y ago
Love the concept of a working memory score for code. > 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 https://www.stashpad.com/blog/working-memory
Izkata 3y ago
First one is far far easier for me to understand for two reasons: * 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.
- webnrrd2k 3y ago
  I found that the meaning of the first one was a lot easier, but there are a lot of assumptions about the called sub-functions... Mainly that they actually behave in a way that's consistent with their names. (I've gotten bitten by that a lot). 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.
  - klauserc 3y ago
    One could argue that the same holds for the second example. Sure, the parameter is called `numbers` but who knows what it will contain at runtime. It could be full of vectors, matrices or sheep. _Some_ of the objects could have a custom definitions for __lt__ and __mod__ (custom overloads for the < and % operators in Python). 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.
    - webnrrd2k 3y ago
      Yes, I guess... if you want to argue that anything could mean anything then it's all bullshit. I don't see how that adds anything to the conversation.
JackFr 3y ago
The big difference is that the first function may be be wrong, but the second is quite obviously wrong. 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.
jacknews 3y ago
"We can then separate out that complexity even further if we move the determination of the value of paid_today into its own function." 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)) This kind of extraction is only useful if you need to re-use the encapsulated calculation IMHO, otherwise just comment each term, or use something more powerful than "if and or then", eg decision tables, etc.
tra3 3y ago
Complexity related -- "Don't wake up the programmer" (https://alexthunder.livejournal.com/309815.html https://alexthunder.livejournal.com/309815.html). 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.
hmmokidk 3y ago
Functional Programming solves this problem.
tialaramex 3y ago
The second function bothers me because it has this arbitrary looking value, presumably as a sentinel, and that's a hidden requirement of the function. This function is actually minimum_odd_number_or_9999 for some reason, and chances are nobody actually wants that. Instead languages should encourage distinguishing None from actual answers - and then sure as an optimisation you'd consider using a sentinel to achieve that mechanically, but it doesn't alter the program meaning. On the other hand, in this style the first program could have any amount of hidden traps.
TRiG_Ireland 3y ago
I've certainly come across the magic number seven plus or minus two before, in study of simultaneous interpreting. The interpreter is usually a sentence or two behind the speaker. The trick, they say, is to increase the size of the chunks, as you can't do much about the number of them. Try to process a sentence in clauses, not in words. (You need to do that anyway, really, as the grammar of the languages may not match up.) I suspect that something similar applies here.
pkkm 3y ago
I don't see any code being improved in this article. The author does the same thing I've noticed Clean Code adherents do: he focuses on the busywork of splitting functions into tiny pieces while missing the actual place that requires the most mental effort to understand and is the most likely to become a source of bugs. In this case, it's the complex boolean expression that relies on Python's operator precedence. It could be rewritten like so: 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 ) ): to make it clearer that the has_passed_probation check only applies in the monthly case.

89 points by gus_leonel 3y ago

Are there any implementations of this? So that it's possible to measure it for a given piece of code?

I have a hard time getting developers to follow this type of advice and a lot of developers will "clean up" and "optimize" the example code in introducing_variable_version by eliminating the paid_today variable back into a single if statement. 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.

Cthulhu_ 3y ago
Yeah that's a constant struggle, developers thinking less and / or clever code is better code; it's self-gratification, and it should be stopped during code review. To them I'd say I'm sorry a series of simple if- statements doesn't challenge you intellectually, but the correctness of these checks are more important than your ego. Of course, there's also a gap in unit tests if they can change an important check without a test failing.
- Ensorceled 3y ago
  It was found during code review AND it revealed a gap in our unit tests. So, 50% success, I guess. Belt and suspenders.
throw1234651234 3y ago
What was bad about it? Did they just do: 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.
- Ensorceled 3y ago
  It was fairly long series of checks, some complicated, and they mixed up an or and not case when merging them into the single if.
  - throw1234651234 3y ago
    Ah, I see now. I think it's a very common mistake. People also mess up negation when they refactor things like these. So your original was if(!loggedIn) return Unauthorized; if(isTokenExpired) return Unauthorized; etc? I think being that explicit about it works pretty well sometimes.
Izkata 3y ago
> I have a hard time getting developers to follow this type of advice 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.

fit2rule 3y ago

[dead]

TexanFeller 3y ago

Having low working memory combined with my working memory slots being overwritten my random digressions my brain decides to pursue, AKA ADHD, reducing the amount of state I need to track is one of the most significant factors for productive coding. I guess that's why I find FP style code that eschews mutable variables and collections and anything that breaks referential transparency(AKA the ability to understand code locally, without surrounding context) appealing.

klauserc 3y ago

Interestingly, I had the exact opposite reaction. I found the second function harder to reason about because it heavily relies on mutation and the fact that individual loop iterations commute with respect to one another. (To understand what the final value of `min_value` is, you first have to understand that `<` being transitive means that the iteration order doesn't matter) 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)".

JackMorgan 3y ago
I agree, I read the first sample almost instantly, and I'm only _pretty sure_ I follow the second one after staring at it for a minute. Changing it to a range+filter+fold would make it an order of magnitude easier to read for me. 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.
Scarblac 3y ago
It depends on how often you have written loops just like that though. To my eye, setting a min_value, then looping through the numbers updating min_value when something lower is found and returning the end result is just a single "chunk" that's immediately obvious. 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".
- deleted 3y ago

quantum_state 3y ago

is there a tool to scan the code and output the working memory metrics for a given cognitive context such as line, etc.?

drittich 3y ago

I'm still left wondering whether the code is correct, in that an employee that is paid weekly does not require passing probation to get paid. Is that the correct logic, or is it the (very common) error of forgetting an extra set of parentheses when using OR? Given that there would probably be more complexity in the real code I would create separate functions expressing the rules for each employee type.

xmcqdpt2 3y ago
Exactly. In a better programming language (like Java!) you could simply use sequential filter steps to avoid all those issues, employees .filter(has_passed_probation) .filter(...) .forEach(p -> run_payroll(p, p.salary)) In general I find those kind of examples useless because the real life version tend to be a lot more complex with dependencies all over the place. All the functions in the article are simple by comparison. In general I don't think it's always better to have six small functions split over four files that are required to do X instead of one 40 line function that has all the logic together.
- throw1234651234 3y ago
  Have fun troubleshooting that. Also, iirc Java just got this in Java 8. Far behind .NET, as usual ;)
  - rufus_foreman 3y ago
    >> Java just got this in Java 8 "just" in this case means as of March 2014.
  - klauserc 3y ago
    I worked on a big Java project where we had to do a lot of list/collection wrangling like that. I found that code using combinator chain/list comprehensions/Streams/LINQ/etc. was significantly less error prone than code with hand-written loops. And that was in a team mostly staffed with junior engineers :)

UglyToad 3y ago

It feels like their calculation missed 2 crucial confounders, stack depth and type tracking. 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.

jagged-chisel 3y ago
Stack depth isn’t an issue for me typically. I assume the function is named correctly and the operation it represents is the operation needed at this point. If I find errors (why did we “getLighterColor” and from the resulting object expect to find a fruit name?), then I investigate the called function to see if it really does stuff a banana into a color and ask its author “wat?” And that’s because the contents of a function the current code depends on should be correct based on its name. 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.
- UglyToad 3y ago
  I think while the name things correctly principle is nice and good to aspire to like anything at the intersection of people doing the right thing and natural language there's a bunch of places it doesn't work. 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.

zellyn 3y ago

I agree with all of this, except the part where they assume extracting a function is essentially free. 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.

thanatropism 3y ago
I mean, since we split systems up because the whole can end up being worth more than the sum of its parts, there's definitely a knowledge of the whole to be kept besides knowledge of the parts. You have to budget for this.
AnimalMuppet 3y ago
> 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. 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.

bluetomcat 3y ago

The "rules" are overly defined with respect to the static elements of source code. Cognitive load in understanding code is not linearly tied to the number of variables, function calls, conditional checks, etc. 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".

thanatropism 3y ago
Isn't variable name length a factor there? 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

heikkilevanto 3y ago

I believe a lot of this is affected about how familiar idioms are used. The second example simplifies to me as something like "Loop to find the smallest", "check parity". The thing that seemed to take most of my cognitive capacity was the unstated assumption that 9999 was clearly bigger than any of the possible numbers. 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)

jagged-chisel 3y ago
> … there must have been a hidden way … 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.”

jagged-chisel 3y ago

> the top function is fairly obviously harder to understand 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.

cborenstein 3y ago

Love the concept of a working memory score for code. > 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 https://www.stashpad.com/blog/working-memory

Izkata 3y ago

First one is far far easier for me to understand for two reasons: * 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.

webnrrd2k 3y ago
I found that the meaning of the first one was a lot easier, but there are a lot of assumptions about the called sub-functions... Mainly that they actually behave in a way that's consistent with their names. (I've gotten bitten by that a lot). 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.
- klauserc 3y ago
  One could argue that the same holds for the second example. Sure, the parameter is called `numbers` but who knows what it will contain at runtime. It could be full of vectors, matrices or sheep. _Some_ of the objects could have a custom definitions for __lt__ and __mod__ (custom overloads for the < and % operators in Python). 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.
  - webnrrd2k 3y ago
    Yes, I guess... if you want to argue that anything could mean anything then it's all bullshit. I don't see how that adds anything to the conversation.

JackFr 3y ago

The big difference is that the first function may be be wrong, but the second is quite obviously wrong. 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.

jacknews 3y ago

"We can then separate out that complexity even further if we move the determination of the value of paid_today into its own function." 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)) This kind of extraction is only useful if you need to re-use the encapsulated calculation IMHO, otherwise just comment each term, or use something more powerful than "if and or then", eg decision tables, etc.

tra3 3y ago

Complexity related -- "Don't wake up the programmer" (https://alexthunder.livejournal.com/309815.html https://alexthunder.livejournal.com/309815.html). 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.

hmmokidk 3y ago

Functional Programming solves this problem.

tialaramex 3y ago

The second function bothers me because it has this arbitrary looking value, presumably as a sentinel, and that's a hidden requirement of the function. This function is actually minimum_odd_number_or_9999 for some reason, and chances are nobody actually wants that. Instead languages should encourage distinguishing None from actual answers - and then sure as an optimisation you'd consider using a sentinel to achieve that mechanically, but it doesn't alter the program meaning. On the other hand, in this style the first program could have any amount of hidden traps.

TRiG_Ireland 3y ago

I've certainly come across the magic number seven plus or minus two before, in study of simultaneous interpreting. The interpreter is usually a sentence or two behind the speaker. The trick, they say, is to increase the size of the chunks, as you can't do much about the number of them. Try to process a sentence in clauses, not in words. (You need to do that anyway, really, as the grammar of the languages may not match up.) I suspect that something similar applies here.

pkkm 3y ago

I don't see any code being improved in this article. The author does the same thing I've noticed Clean Code adherents do: he focuses on the busywork of splitting functions into tiny pieces while missing the actual place that requires the most mental effort to understand and is the most likely to become a source of bugs. In this case, it's the complex boolean expression that relies on Python's operator precedence. It could be rewritten like so: 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 ) ): to make it clearer that the has_passed_probation check only applies in the monthly case.