6 ms·

I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. Doesn't matter how it looks. If its not possible

by zwnow 2y ago

I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. 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.

ajross 2y ago
> I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. 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.
- zwnow 2y ago
  I even wrote no matter how it looks? I meant the goal of your function needs to be grasped within a reasonable amount of time. This works for every codebase.
  - ajross 2y ago
    > 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/utils/rb.c#L153 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.
    - sunrunner 2y ago
      This is exactly the kind of example I have in my head for code that constitutes a high level of information density. Adding abstraction and 'literate' constructs to try and make things readable is ultimately deferring the fact that understanding the code here is fundamentally about understanding a specific implementation of an algorithm, and to understand _that_ ultimately needs the reader to build their own clear mental model of both the algorithm itself and how it's been translated into a specific form. 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.
      - whstl 2y ago
        > 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 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.
- necrotic_comp 2y ago
  I agree with this up to a point - having consistent code style with some sort of formatter (gofmt, black, clang-format) goes a long way to reducing complexity of understanding because it unifies visual style. 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.
  - ajross 2y ago
    > While there is room for op-eds in the paper, 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.
jacobr1 2y ago
There is a call-stack depth problem here that is specific to codebases though. For one familiar with the the conventions, key data abstractions (not just data model but convention of how models are structured and relate) and key code abstractions, a well formed function is easy to understand. But someone relatively new to the codebase will need to take a bunch of time switching between levels to know what can be assumed about the state or control flow of the system in the context of when that function/subroutine is running. Better codebases avoid side-effects, but even with good separation there, non-trivial changes require strong reasoning about where to make changes in the system to avoid introducing side-effects and not just passing extra state or around all over the place. 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.
- zwnow 2y ago
  But who talked about bad architecture? Good readable code doesn't rule out good architecture. Surely some things are complicated but even then, a dev should be able to quickly see whats going on with minimal expertise in a codebase.
  - swatcoder 2y ago
    They're suggesting that readability and "5-second accessibility" are essentially contextual and build on a conceptual language that might be specific to a tool, team, or 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.
sunrunner 2y ago
Does this apply to all domains and all 'kinds' of code? 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.
- zwnow 2y ago
  If the algorithm is well known, it's all good as long as the function name for it is somewhat understandable. I have to work with 200 line functions at work and it's a complete, excuse the language, shitshow.
  - sunrunner 2y ago
    > as long as the function name for it is somewhat understandable 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...
    - zwnow 2y ago
      My original comment never was about understanding the implementation details. It was about understanding the high level goal of the function.
andybp85 2y ago
for the codebase i work on, i made a rule that "functions do what the name says and nothing else". this way if the function does too much, hopefully you feel dumb typing it and realize you should break it up.
- quinnirill 2y ago
  Then what does the function that calls the split functions get called? foo_and_bar_and_qoo? And if they’re called only under some conditions?
  - brulard 2y ago
    It likely has some higher-level meaning other than just do foo, bar, qoo. 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.
    - quinnirill 2y ago
      Yeah, I agree in spirit but I think the answer is more ”it depends” than something where you should feel bad or something if you deviate from it. If washDishes also sends a bunch of metrics/diagnostics or updates a database of your favorite dish washing programs somewhere inside it, that’s probably fine. Otherwise you push the path of least resistance to just be vague instead, then you get a codebase full of functions with names like handle or process.
  - jbeninger 2y ago
    I have definitely been guilty of naming functions foo_thenBarSometimes. I wince whenever I write them, but I've never really regretted seeing them later on, even after years. So sometimes it really is a perfectly good name. Sometimes there are two related functions that are often called together and don't have a succinct label for the combined operation.
intrasight 2y ago
> Doesn't matter how it looks. That's the mindset that the author is trying to counter.
callc 2y ago
Generally agree. 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.
jayd16 2y ago
So it should take 5 minutes whether it's your language or choice or the assembly it compiles to? Or does it matter how it looks in _that_ case?
jimmaswell 2y ago
This is what xmldoc/jsdoc/etc are for. If it's not 100% obvious from the name, put a summary of the function's assumptions, side effects, output, and possibly an example in the comment-doc. If you do this right, the next programmer will never have to read your source at all (or even navigate to your file! They'll hover over a method call or find it in the dot-autocomplete and see a little tootip with this documentation in it, and know all they need to know). It's an incredible thing when it works. It's a little bit more effort but I don't accept the FUD around "comments become out of date immediately because the code will change" etc. - that should be part of code review. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/ https://learn.microsoft.com/en-us/dotnet/csharp/language-ref...
- hinkley 2y ago
  Always, always check in about whether it would be simpler to fix the function than to write an extended apology for it working the way it does. 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.
fasbiner 2y ago
Sounds like you are content to limit yourself to problems that do not contain more irreducible complexity or require more developer context than what fits within five seconds of comprehension. 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.
- hinkley 2y ago
  > is an ex cathedra metaphysical claim 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.
  - fasbiner 2y ago
    If you find compact language above your level of proficiency confusing, you can literally ask an LLM to explain it for you to trade efficiency for accessibility. 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.
bad_haircut72 2y ago
Do you think you would understand every function in the doom codebase in under 5 seconds? Is this bad proframming then?
- hinkley 2y ago
  Making code faster without making it more difficult to read is an art so black that some people insist it doesn’t exist. Doom is about being fast. Doom famously has a function in it so obscure that nobody remembers how they even came up with it.
- zwnow 2y ago
  You did not understand my original comment... Doom might be good code but its maintainability is horrendous in modern standards.
hinkley 2y ago
Code that looks like it has a bug in it but doesn’t will draw the eye over, and over, and over again when fishing for how regressions or bugs got into the code. This is the real cost of code smells. At some point it’s cheaper for me to clean up your mess than to keep walking past it every day. But I’m going to hate you a little bit every time I do.

by zwnow 2y ago

ajross 2y ago

> I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. 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.

zwnow 2y ago
I even wrote no matter how it looks? I meant the goal of your function needs to be grasped within a reasonable amount of time. This works for every codebase.
- ajross 2y ago
  > 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/utils/rb.c#L153 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.
  - sunrunner 2y ago
    This is exactly the kind of example I have in my head for code that constitutes a high level of information density. Adding abstraction and 'literate' constructs to try and make things readable is ultimately deferring the fact that understanding the code here is fundamentally about understanding a specific implementation of an algorithm, and to understand _that_ ultimately needs the reader to build their own clear mental model of both the algorithm itself and how it's been translated into a specific form. 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.
    - whstl 2y ago
      > 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 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.
necrotic_comp 2y ago
I agree with this up to a point - having consistent code style with some sort of formatter (gofmt, black, clang-format) goes a long way to reducing complexity of understanding because it unifies visual style. 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.
- ajross 2y ago
  > While there is room for op-eds in the paper, 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.

jacobr1 2y ago

There is a call-stack depth problem here that is specific to codebases though. For one familiar with the the conventions, key data abstractions (not just data model but convention of how models are structured and relate) and key code abstractions, a well formed function is easy to understand. But someone relatively new to the codebase will need to take a bunch of time switching between levels to know what can be assumed about the state or control flow of the system in the context of when that function/subroutine is running. Better codebases avoid side-effects, but even with good separation there, non-trivial changes require strong reasoning about where to make changes in the system to avoid introducing side-effects and not just passing extra state or around all over the place. 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.

zwnow 2y ago
But who talked about bad architecture? Good readable code doesn't rule out good architecture. Surely some things are complicated but even then, a dev should be able to quickly see whats going on with minimal expertise in a codebase.
- swatcoder 2y ago
  They're suggesting that readability and "5-second accessibility" are essentially contextual and build on a conceptual language that might be specific to a tool, team, or 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.

sunrunner 2y ago

Does this apply to all domains and all 'kinds' of code? 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.

zwnow 2y ago
If the algorithm is well known, it's all good as long as the function name for it is somewhat understandable. I have to work with 200 line functions at work and it's a complete, excuse the language, shitshow.
- sunrunner 2y ago
  > as long as the function name for it is somewhat understandable 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...
  - zwnow 2y ago
    My original comment never was about understanding the implementation details. It was about understanding the high level goal of the function.

andybp85 2y ago

for the codebase i work on, i made a rule that "functions do what the name says and nothing else". this way if the function does too much, hopefully you feel dumb typing it and realize you should break it up.

quinnirill 2y ago
Then what does the function that calls the split functions get called? foo_and_bar_and_qoo? And if they’re called only under some conditions?
- brulard 2y ago
  It likely has some higher-level meaning other than just do foo, bar, qoo. 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.
  - quinnirill 2y ago
    Yeah, I agree in spirit but I think the answer is more ”it depends” than something where you should feel bad or something if you deviate from it. If washDishes also sends a bunch of metrics/diagnostics or updates a database of your favorite dish washing programs somewhere inside it, that’s probably fine. Otherwise you push the path of least resistance to just be vague instead, then you get a codebase full of functions with names like handle or process.
- jbeninger 2y ago
  I have definitely been guilty of naming functions foo_thenBarSometimes. I wince whenever I write them, but I've never really regretted seeing them later on, even after years. So sometimes it really is a perfectly good name. Sometimes there are two related functions that are often called together and don't have a succinct label for the combined operation.

intrasight 2y ago

> Doesn't matter how it looks. That's the mindset that the author is trying to counter.

callc 2y ago

Generally agree. 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.

jayd16 2y ago

So it should take 5 minutes whether it's your language or choice or the assembly it compiles to? Or does it matter how it looks in _that_ case?

jimmaswell 2y ago

This is what xmldoc/jsdoc/etc are for. If it's not 100% obvious from the name, put a summary of the function's assumptions, side effects, output, and possibly an example in the comment-doc. If you do this right, the next programmer will never have to read your source at all (or even navigate to your file! They'll hover over a method call or find it in the dot-autocomplete and see a little tootip with this documentation in it, and know all they need to know). It's an incredible thing when it works. It's a little bit more effort but I don't accept the FUD around "comments become out of date immediately because the code will change" etc. - that should be part of code review. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/ https://learn.microsoft.com/en-us/dotnet/csharp/language-ref...

hinkley 2y ago
Always, always check in about whether it would be simpler to fix the function than to write an extended apology for it working the way it does. 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.

fasbiner 2y ago

Sounds like you are content to limit yourself to problems that do not contain more irreducible complexity or require more developer context than what fits within five seconds of comprehension. 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.

hinkley 2y ago
> is an ex cathedra metaphysical claim 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.
- fasbiner 2y ago
  If you find compact language above your level of proficiency confusing, you can literally ask an LLM to explain it for you to trade efficiency for accessibility. 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.

bad_haircut72 2y ago

Do you think you would understand every function in the doom codebase in under 5 seconds? Is this bad proframming then?

hinkley 2y ago
Making code faster without making it more difficult to read is an art so black that some people insist it doesn’t exist. Doom is about being fast. Doom famously has a function in it so obscure that nobody remembers how they even came up with it.
zwnow 2y ago
You did not understand my original comment... Doom might be good code but its maintainability is horrendous in modern standards.

hinkley 2y ago

Code that looks like it has a bug in it but doesn’t will draw the eye over, and over, and over again when fishing for how regressions or bugs got into the code. This is the real cost of code smells. At some point it’s cheaper for me to clean up your mess than to keep walking past it every day. But I’m going to hate you a little bit every time I do.