What makes code hard to read: Visual patterns of complexity (2023)

To me the functional style is much more easy to parse as well. Maybe the lesson is that familiarity can be highly subjective.

I for example prefer a well chosen one-liner list comprehension in python over a loop with temporary variables and nested if statements most of the time. That is because usually people who use the list comprehension do not program it with side effects, so I know this block of code, once understood stands for itself.

The same is true for the builder style code. I just need to know what each step does and I know what comes out in the end. I even know that the object that was set up might become relevant later.

With the traditional imperative style that introduces intermediate variables I might infer that those are just temporary, but I can't be sure until I read on, keeping those variables in my head. Leaving me in the end with many more possible ways future code could play out. The intermediate variables have the benefit of clarifying steps, but you can have that with a builder pattern too if the interface is well-chosen (or if you add comments).

This is why in an imperative style variables that are never used should be marked (e.g. one convention is a underscore-prefix like _temporaryvalue — a language like Rust would even enforce this via compiler). But guess what: to a person unfamilar with that convention this increases mental complexity ("What is that weird name?"), while it factually should reduce it ("I don't have to keep that variable in the brain head as it won't matter in the future").

In the end many things boil down to familiarity. For example in electronics many people prefer to write a 4.7 kΩ as 4k7 instead, as it prevents you from accidentally overlooking the decimal point and making an accidental off-by-a-magnitude-error. This was particularly important in the golden age of the photocopier as you can imagine. However show that to a beginner and they will wonder what that is supposed to mean. Familiarity is subjective and every expert was once a beginner coming from a different world, where different things are familiar.

Something being familiar to a beginner (or someone who learned a particular way of doing X) is valuable, but it is not necessarily an objective measure of how well suited that representation is for a particular task.

Agreed —

seeinglogic's article made me think of a 3rd option:

1. Sorta long functional chain where the type changes partway through 2. Use temp variables 3. (New option) Use comments

(Here's funcA from seeinglogic's article, but I added 3 comments)

    function funcC(graph) {
      return 
        // target node
        graph.nodes(`node[name = ${name}]`)
          // neighbor nodes
          .connected()
          .nodes()
          // visible names
          .not('.hidden')
          .data('name');
     }

    function funcB(graph) {
      const targetNode = graph.nodes(`node[name = ${name}]`)
      const neighborNodes = targetNode.connected().nodes();
      const visibleNames = neighborNodes.not('.hidden').data('name')

      return visibleNames;
    }

The dig on chains of map/reduce/filter was listed as a "Halstead Complexity Takeaway", and seemed to come out of the blue, unjustified by any of the points made about Halstead complexity. In fact in your later funcA vs. funcB example, funcB would seem to have higher Halstead complexity due to its additional variables (depending on whether they count as additional "operands" or not). In general, long chains of functions seem like they'd have lower Halstead complexity.

The "anti-functional Tourette's" comment was partly a response to how completely random and unjustified it seemed in that part of the article, and also that this feels like a very common gut reaction to functional programming from people who aren't really willing to give it a try. I'm not only arguing directly against you here, but that attitude at large.

Your funcA vs. funcB example doesn't strike me as "functional" at all. No functions are even passed as arguments. That "fluent" style of long chains has been around in OO languages for a while, independent of functional programming (e.g. see d3.js*, which is definitely not the oldest). Sure, breaking long "fluent" chains up with intermediate variables can sometimes help readability. I just don't really get how any of this is the fault of functional programming.

I think part of the reason funcB seems so much more readable is that neither function's name explains what it's trying to do, so you go from 0 useful names to 3. If the function was called "getNamesOfVisibleNeighbors" it'd already close the readability gap a lot. Of course if it were called that, it'd be more clear that it might be just trying to do too much at once.

I view the "fluent" style as essentially embedding a DSL inside the host language. How readable it is depends a lot on how clear the DSL itself is. Your examples benefit from additional explanation partly because the DSL just seems rather inscrutable and idiosyncratic. Is it really clear what ".data()" is supposed to do? Sure, you can learn it, but you're learning an idiosyncrasy of that one library, not an agreed-upon language. And why do we need ".nodes()" after ".connected()"? What else can be connected to a node in a graph other than other nodes? Why do you need to repeat the word "node" in a string inside "graph.nodes()"? Why does a function with the plural "nodes" get assigned to a singular variable? As an example of how confusing this DSL is, you've claimed to find "visibleNames", but it looks to me like you've actually found the names of visible neighborNodes. It's not the names that are not(.hidden), it's the nodes, right? Consider this:

    function getVisibleNeighborNames(graph) {
        return graph
            .nodeByName(name)
            .connectedNodes()
            .filter(node => !node.isHidden)
            .map(node => node.name)
    }

Getting the DSL right is really hard, which only increases the benefit of using things like "map" and "filter" which everyone immediately understands, and which have no extrinsic complexity at all.

You could argue that it's somehow "invalid" to change the DSL, but my point is that if you're using the wrong tool for the job to begin with, then any further discussion of readability is in some sense moot. If you're doing a lot of logic on graphs, you should be dealing with a graph representation, not CSS classes and HTML attributes. Then the long chains are not an issue at all, because they read like a DSL in the actual domain you're working in.

*Sidenote: I hate d3's standard style, for some of the same reasons you mention, but mainly because "fluent" chains should never be mutating their operand.

>For me, roughly 5 calls in a chain is where things begin to become harder to read, which is the length of the example I used.

This isn't just about readability. Chaining or FP is structurally more sound. It is the more proper way to code from a architectural and structural pattern perspective.

     given an array of numbers

   1. I want to add 5 to all numbers
   2. I want to convert to string
   3. I want to concat hello
   4. I want to create a reduced comma seperated string
   5. I want to capitalize all letters in the string. 

   // assume x is the array
   acc = ""

   for(var i = 0, i < x.length; x++) {
       value = x[i] + 5
       value += 5
       stringValue = str(value).concat(hello)
       acc += stringValue + ","
   }

   for (var i = 0, i < acc.length; i++) {
       acc[i] = capitalLetter(acc[i])
   }

    addFive(x) = [i + 5 for i in x]
    toString(x) = [str(i) for i in x]
    concatHello = [i + "hello" for i in x]
    reduceStrings(x) = reduce((i, acc) = acc + "," + i, x)
    capitalize(x) = ([capitalLetter(i) for i in x]).toString()

Mind you that I know you're thinking about chaining. Chaining is eqivalent to inlining multiple operations together. So for example in that case

     x.map(...).map(...).map(...).reduce(...).map(...)

     //can be made into
     addFive(x) = x.map(...)
     toString(x)= x.map(...)
     ...

It's not about going overboard here. The FP simply needs to be formatted to be readable, but it is the MORE proper way to code to make your code modular general and decoupled.

You don't ever need to "understand everything at once". You can read each stanza linearly. The for loop style is the approach where everything often needs to be understood all at once since the logic is interspersed throughout the entire body.

In a practical example you'd create a named intermediate type which becomes a new base for reasoning. Once you convinced yourself that the first part of the chain responsible for creating that type (or a collection of it) is correct, you can forget it and free up working memory to move on to the next part. The pure nature of the steps also makes them trivially testable as you can just call them individually with easy to construct values.

If you assign an intermediate result to a variable in a procedural loop, you can also assign intermediate results of parts of this chain to variables.

In fairness, if this was in a relational data store, the same code as above would probably look more like...

SELECT DISTINCT authors.some_field FROM books JOIN authors ON books.author_id = authors.author_id WHERE books.pageCount > 1000

And if you wanted to grab the entire authors record (like the code does) you'd probably need some more complexity in there:

SELECT * FROM authors WHERE author_id IN ( SELECT DISTINCT authors.author_id FROM books JOIN authors ON books.author_id = authors.author_id WHERE books.pageCount > 1000 )

Scrolled to find the SQL. Such an elegant, powerful language. Really happy I chose SQLite for my game/project.

This is 5 times more readable than FP example above for the same computation. The FP example uses variable book(s) five times, where using it once was sufficient for SQL. Perhaps FP languages could have learned something from SQL...

Notably this example is declarative, the original is functional, and neither is imperative.

Folks don't seem to have a problem when SQL does it. Only when code like Pandas does it...

What is "long"?

I had a similar experience. I was a lead on a project where the client sent a functional expert to literally (at times) watch over my shoulder as I worked. He got very frustrated after a few weeks, seeing little in the way of code being laid down. He even complained to the project manager. That's because this was a complicated manufacturing system, and I was absorbing the necessary rules for it and designing it... a process that involved mostly sitting and thinking.

When I decided on the final design and basically barfed all the code out in a matter of days, I walked this guy (a non-programmer) through the code. He then wrote my manager a letter declaring it to be the "most beautiful code he had ever seen." I still have the Post-It she left in my cube telling me that.

I have little tolerance for untidy code, and also overly-clever syntax that wastes the reader's time trying to unravel it.

And now we have languages building more inconsistent and obscure syntax in as special-case options, wasting more time. Specifically I'm thinking about Swift; where, if the last parameter in a function call is a closure, it's a "trailing" closure and you can just ignore the function signature and plop the whole closure right there AFTER the closing parenthesis. Why?https://www.hackingwithswift.com/sixty/6/5/trailing-closure-...

This is just one example, and yeah... you can get used to it. But in this example, the language has undermined PARENTHESES, a notation that is almost universally understood to enclose things. When something that basic is out the window, you're dealing with language designers who lack an appreciation for human communication.

> The functions are self contained in that their dependencies/inputs are the arguments provided or other pure functions and the outputs are entirely in the return type.

Is this just a fancy way of saying static functions?

There’s a difference between simplifying a concept and stating it plainly.

I use this analogy a lot. Code can be like a novel, a short story, or a poem. A short story has to get to the point pretty quickly. A poem has to be even more so, but it relies either on shared context or extensive unpacking to be understood. It’s beautiful but not functional.

And there are a bunch of us short story writers who just want to get to the fucking point with a little bit of artistic flair, surrounded by a bunch of loud novel and mystery writers arguing with the loudest poets over which is right when they are both wrong. And then there’s that asshole over there writing haikus all the fucking time and expecting the rest of us to be impressed. The poets are rightfully intimidated but nobody else wants to deal with his bullshit.

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.

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.

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

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.

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.

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.

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.

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?

Well of course if you have the freedom to write a mathematical expression you're going to be able to present it in a way that is clearer than if you have to type monospace characters into a text editor.

I'm not sure it's realistic to expect to be able to type a mathematical expression using ascii more clearly than you can write it by hand (or implement using special unicode characters).

I understand that you might find the mathematical notation clearer but I think it's presumptuous of you to speak on behalf of all humans, or even all human mathematicians. I'm a mathematics graduate and I find the conditional operator more readable in a program because it corresponds to what the program actually does (it checks the condition first); but I also recognize that the two notations have exactly the same information content and only differ superficially in syntax, making it entirely a matter of familiarity.

I love code golf as much as anyone, not sure it's worth it on such small methods tho. Any of the propositions would be fine. Anyway:

    def oddness(n):
      return ["Even", "Odd"][n % 2]

This is, IMHO, the idiomatic way to do so.

Since the article was in JavaScript:

  const getOddness = (n) => (
    n % 2
      ? 'Odd'
      : 'Even'
  )

While this is simple and all, the English words if/else don’t require the reader to know the ?: convention. Depending on what background the reader may have, they could think of the set notation where it could mean “all the evens such that odd is true” which makes no sense. Its also very close to a key:value set notation. If/else leave no doubts for the majority of readers. It’s more inclusive if you will.

Putting “return” on a different line from the actual value you’re returning?

In case of handling exceptions (and similar stuff), I also prefer avoiding unnecessary nesting.

For two (or more) equally valid, I prefer keeping same nesting.

I want it on the other side (on the function return) so that it's consistently displayed in type hints and intellisense so I don't have to navigate the code backwards 3-4 layers to find the root type (do you see what I'm saying?)

    function checkDogs(dogs: Dog[]) : DogBreedAndSize[] {
      return dogs.map(d => /* ... */)
    }

I'd say on backend, my preference is statically something like C#. Statically typed but enough type flexibility to be interesting (tuples, anonymous types, inferred types, etc)

What’s tragic is this is completely self-inflicted and you could argue is done against the kind of code structure that is more idiomatic to C#. Luckily, you don’t have to do it if you are not already working with this type of codebase.

As an example, for my first decade of programming I worked on code where the coding style banned the ?: operator, so I didn't use it and found such code hard to read the few times I encountered it. Then I got a new job where one of the programmers really liked that operator and so I was forced to learn how to read it, now such code is more readable then the if statements to me - when used in the way we use it on this project.

Another thing that comes to mind is the level at which one is familiar with a particular style of code organization & a set of abstractions. For example, Rails devs really have absolutely no problem getting up to speed with any given Rails app, but people who don't practice Ruby/Rails as their primary language/framework often complain how complicated and magical it is.

Poor abstractions. Good abstractions make it easier to change things, by decomposing the code into cohesive pieces with low coupling so that you can swap them out and having to think about the surrounding pieces beyond their interfaces. A good interface is small and logical.

AST level would have to automatically figure out what parts should be aligned, an alternative is to keep saving it in text but tweak the meaning of the "tab" character, so the developer still has control over what gets aligned: https://nick-gravgaard.com/elastic-tabstops/

Not great since viewing it in something that doesn't understand elastic tabstops would just be a mess, but it solves one of the issues the other response brings up, and I think some sort of user control like that is going to remain necessary either way.