> Chaining together map/reduce/filter and other functional programming constructs (lambdas, iterators, comprehensions) may be concise, but long/multiple chains hurt readability

This is not at all implied by anything else in the article. This feels like a common "I'm unfamiliar with it so it's bad" gripe that the author just sneaked in. Once you become a little familiar with it, it's usually far easier to both read and write than any of the alternatives. I challenge anyone to come up with a more readable example of this:

    var authorsOfLongBooks = books
        .filter(book => book.pageCount > 1000)
        .map(book => book.author)
        .distinct()

By almost any complexity metric, including his, this code is going to beat the snot out of any other way of doing this. Please, learn just the basics of functional programming. You don't need to be able to explain what a Monad is (I barely can). But you should be familiar enough that you stop randomly badmouthing map and filter like you have some sort of anti-functional-programming Tourette's syndrome.

    authors_of_long_books = set()

    for book in books:
        if len(book.pages) > 1000:
            authors_of_long_books.add(book.author)

    return authors_of_long_books

    authors_of_long_books: set[Author] = {book.author for book in books if book.page_count > 1000}

There is a (large, I believe) aspect of good code that is fundamentally qualitative & almost literary. This annoys a lot of computer programmers (and academics) who are inclined to the mathematical mindset and want quantitative answers instead.

I love dostoyevsky and wodehouse, both wrote very well, but also very differently. While I don't think coding is quite that open a playing field, I have worked on good code bases that feel very different qualitatively. It often takes me a while to "get" the style of a code base, just as a new author make take a while for me to get.

  function isReadyToDoThing(Foo foo) {
    return foo.ready
  }

  function processStuff((Foo foo) {
    if isReadyToDoThing(foo) {
      res = workflowA(foo)
      res2 = workflowB(foo)
      return res && res2
    }
  }

  function isReadyToDoThing(Foo foo) {
    return foo.attribute1 && foo.attribute2 && ! otherThing(foo)
  }

         ⎧  'Even' n mod 2 = 0
  f(n) = ⎨
         ⎩  'Odd'  n mod 2 ≠ 0

The article's good, but misses my most mentally-fatiguing issue when reading code: mutability.

It is such a gift to be able to "lock in" a variable's meaning exactly once while reading a given method, and to hold it constant while reasoning about the rest of the method.

Your understanding of the method should monotonically increase from 0% to 100%, without needing to mentally "restart" the method because you messed up what the loop body did to an accumulator on a particular iteration.

This is the real reason why GOTOs are harmful: I don't have a hard time moving my mind's instruction-pointer around a method; I have a hard time knowing the state of mutable variables when GOTOs are in play.

Maybe it's just me, but TypeScript makes code hard to read.

It's fine if the data model is kept somewhat "atomic" and devs are diligent about actually declaring and documenting types (on my own projects, I'm super diligent about this).

But once types start deriving from types using utility functions and then devs slack and fall back to type inference (because they skip an explicit type), it really starts to unravel because it's very hard to trace fields back to their origin in a _deep_ stack (like 4-5 levels of type indirection; some inferred, some explicit, some derived, some fields get aliased...).

    type Dog = {
      breed: string
      size: "lg" | "md" | "sm"
      // ...
    }

    type DogBreedAndSize = Pick<Dog, "breed" | "size">

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

    const checkedDoggos = checkDogs([])

Versus:

    function checkDogs(dogs: Dog[]) {
      // ...
    }

Very subtle, but for large data models with deep call stacks, the latter is completely unusable and absolutely maddening.

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

My pet peeve:

    function getOddness4(n: number):
      if (n % 2 === 0):
        return "Even";
      return "Odd";

While it is shorter, I prefer vastly prefer this one:

    function getOddness2(n: number):
      if (n % 2 === 0):
        return "Even";
      else:
        return "Odd";

Reason: getOddness4 gives some sense of asymmetry, whereas "Even" and "Odd" are symmetric choices. getOddness2 is in that respect straightforward.

  function getOddness(n: number)
    return (n % 2 === 0)
      ? "Even"
      : "Odd";

This is interesting. Something I long wondered about Lisp code, was how having glyphs that are angled (parenthesis) without much indentation in many cases might be difficult to read just because of the visual aspects of it.

     (((
       ((
         (

Takes some staring at to figure out what's what.

I my view, code complexity is best expressed in the size of it's syntax tree, with maybe an additional term for the number of unique nodes. The real mistake here is the assumption that local reductions in complexity make a meaningful difference to overall complexity. Small local decreases in complexity may guide you towards the local minimum of complexity, but will never substantially change the complexity of the code-base overall. All measurements of code complexity are essentially as good as asking "how much code do you have".

I've never understood the hate for variable shadowing. Maybe it's because I mostly use Rust, but I've always found it a useful boon for readability. You often want to extract/parse/wrap/package some value within the middle of a function in a manner that changes its type/form but not its semantic purpose. Shadowing the old value's variable name is brilliant: it communicates that there's a step-change in the responsibilities of the function, demarcating layers from one-another and preventing accidental use of the old value.

> For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using a well-named variable

> Is the second one marginally less efficient?

> Yes.

No, both versions are just as efficient:

In both versions, the same objects are allocated, stored on the heap, and garbage collected. The difference in efficiency comes down to the compiler.

For the second version, the compiler should observe that each variable is only used immediately after it's declared, and thus treat those objects as "out-of-scope" as if it's a chained function call.

Towards the end he had an example of splitting a sequence of "graph.nodes(`node[name = ${name}]`).connected().nodes().not('.hidden').data('name');" adding variable between some of the . in there and claimed it was marginally less efficient. This is sometimes true, but if it ever is you need to talk to your tool vendors about a better optimizes. If you are working in a language without an optimizer than the marginal difference from that optimization applied by hand will be far smaller than the performance improvements you will get by rewriting in a language with an optimizer. Either way, readability trumps performance: either because the performance is the same, or if performance mattered you would have choosen a different language in the first place.

Kudos to seeinglogic for trying to quantify that “readablity” is. We need a lot more of this. (I feel like the most common definition of readability in use today is “readable to me“.)

I have a half-baked thought that we could find the actual dimensions of readability if we gave a test to a very large group of people and asked them to pick a sentence that describes what the code does. Each question would be timed. The questions that most people answered correctly in the shortest average time would provide us with examples of “real-world readable” code, and more importantly, might help us identify some truly not-readable practices.

I predict we’d see respondents start to cluster around various things, like “how long have they been programming?“, “do they understand programming paradigm X?“, etc. Perhaps the results would shift over time, as various things came into and out of fashion.

> there’s a better chance that the programmer forgets to properly handle all of the possibilities

Author totally forgot about IDEs. Yes, I know some coders frown upon IDEs and even color coding (like Rob Pike), but any modern code editor will shout loudly about an unhandled null-pointer check.

Also depends on the language, e.g. it's less reliable in Javascript and Python, but in static typed languages it's pretty obvious at no additional cognitive load.

I always shrugged off the concept of code metrics (from LoCs to coverage) as a distraction from getting actual things done. But since doing more code-review I started to lack a framework to properly explain why a particular piece of code smells. I sympathize with the way the author cautiously approaches any quantitative metrics and talks of them more like heuristics. I agree that both Halstead Complexity and Cognitive Complexity are useless as absolute values. But they can be brought up in a conversation about a potential refactoring for readability.

What I didn't find is a mention of a context when reading a particular function. For example, while programming in Scala I was burnt more than once by one particular anti-pattern.

Suppose you have a collection of items which have some numerical property and you want a simple sum of that numbers. Think of shopping cart items with VAT tax on them, or portfolio positions each with a PnL number. Scala with monads and type inference makes it easy and subjectively elegant to write e.g.

  val totalVAT = items.map(_.vat).sum

But if `items` were a `Set[]` and some of the items happened to have the same tax on them, you would get a Set of numbers and a wrong sum in the end.

You could append to the list of such things until the OutOfMemoryError. But it's such a beautiful and powerful language. Sigh.

Do people really really agree with "Shorthand constructs that combine statements decreases difficulty"? The author even identifies a problem with the example from the original guide.

I think things like Halstead complexity or cyclomatic complexity are more heuristic than law. To read code, the most important thing to me is the abstractions that are built, and how effectively they bury irrelevant complexities and convey important concepts.

As an example, I recently refactored some Java code that was calling a service that returned a list of Things, but it was paged: You might have to make multiple calls to the service to get all the Things back. The original code used a while loop to build a list, and later in the same function did some business logic on the Things. My refactoring actually made things more complex: I created a class called a Spliterator that iterated through each page, and when it was exhausted, called the service again to get the next one. The upside was, this allowed me to simply put the Things in a Stream<Thing> and, crucially, buried the paged nature of the request one level deeper. My reasoning is that separating an implementation detail (the request being paged) from the business logic makes the code easier to read, even if static code analysis would rate the code as slightly more complex. Also, the code that handles the pages is fairly robust and probably doesn't need to be the focus of developer attention very often, if ever, while the code that handles the business logic is much more likely to need changes from time to time.

As programmers, we have to deal with a very long chain of abstractions, from CPU instructions to network calls to high-falutin' language concepts all the way up to whatever business logic we're trying to implement. Along the way, we build our own abstractions. We have to take care that the abstractions we build benefit our future selves. Complexity measures can help measure this, but we have to remember that these measures are subordinate to the actual goal of code, which is communicating a complex series of rules and instructions to two very different audiences: The compiler/interpreter/VM/whatever, and our fellow programmers (often, our future selves who have forgotten half of the details about this code). We have to build high-quality abstractions to meet the needs of those two audiences, but static code analysis is only part of the puzzle.

This talks about the "what" of the code, but you have to also convey the "why".

If you have a well understood problem space and a team that is up to speed on it, then the "why" is well established and the code is the "what".

However, there are often cases where the code is capturing a new area that isn't fully understood. You need to interleave an education of the "why" of the code.

I was once asked to clean up for release 10k lines of someone else's robotics kinematics library code. There weren't any comments, readmes, reference programs, or tests. It was just impenetrable, with no starting point, no way to test correctness, and no definition of terms. I talked to the programmer and he was completely proud of what he had done. The variable names were supposed to tell the story. To me it was a 10k piece puzzle with no picture! I punted that project and never worked with that programmer again.

> To bring closure to the story at the beginning of this post, the codebase that was breaking my brain had several anti-patterns, specifically related to items 1, 2, 3, 6, and 8 in the list above.

FYI: If you get into this situation in C#, the .editorconfig file and the "dotnet format" command are a godsend.

I inherited a very large, and complicated C# codebase with a lot of "novice" code and inconsistent style. I spent about 3 weeks adding rules to .editorconfig, running "dotnet format" and then manually cleaning up what it couldn't clean up. Finally, I added a Github Action to enforce "dotnet format" in all pull requests.

As a result: 1: The code was significantly more readable. 2: It trapped mistakes from everyone, including myself.

There are a few areas where we have to disable a rule via #pragma; but these are the exception, not the norm.

The notion of "readable code" involves 2 parties:

a) The skill level of the person who produces the code

b) The skill level of the person who reads the code.

Most of the time, we tend to blame the a) person.

I think the only one I disagree with here is the function chains example. I may agree with a different example, but with this one I find the chained version without variables much easier to understand because I'm traversing the graph visually in my head, while the variables are additional state I have to keep track of in my head.

----

Really I was hoping this would be about actual visual patterns and not syntax. It's my major issue with how strict code linting/autoformatting is nowadays.

For example, the "black" formatter for python requires this:

    drawer.ellipse((10, 10, 30, 30), fill=(256, 256, 0))
    drawer.ellipse((370, 10, 390, 30), fill=(256, 256, 0))
    drawer.arc((20, 0, 380, 180), 15, 165, fill=(256, 256, 0), width=5)

The first argument is (x1, y1, x2, y2) of a bounding box and black wants to align x1 for "ellipse" with y1 of "arc". Do people really find that more readable than this?

    drawer.ellipse( (10,  10,  30,  30), fill=(256, 256, 0) )
    drawer.ellipse( (370, 10, 390,  30), fill=(256, 256, 0) )
    drawer.arc(     (20,   0, 380, 180), 15, 165, fill=(256, 256, 0), width=5 )

Or perhaps something more common in python, kwargs in function arguments. No spacing at all is standard python style:

    Foobar.objects.create(
        name=form.name,
        owner=user,
        location=form.location,
        source=form.source,
        created=time.now,
    )

Instead for me it's much easier to read this, since I don't have to scan for the "=" and mentally parse each line, it's just a table:

    Foobar.objects.create(
        name        = form.name,
        owner       = user,
        location    = form.location,
        source      = form.source,
        created     = time.now,
    )

But no linters/formatters accept such spacing by default. I think flake8 (linter) could be configured to ignore whitespace like this, but black (formatter) can't.

I find the main problem is not wanting to split up functions.

I greatly prefer small helper functions, so that a more complicated one becomes more readable.

Even declaring a little local variable which explains in English what the condition you’re going to test is supposed to do is greatly appreciated

Why not apply a programming methodology which allows one to leverage a rich set of tools which were created for making text more readable and visually pleasing?

http://literateprogramming.com/

For a book-length discussion of this see:

https://www.goodreads.com/book/show/39996759-a-philosophy-of...

previously discussed here at: https://news.ycombinator.com/item?id=27686818 (and other times in a more passing mention --- "Clean Code" adherents should see: https://news.ycombinator.com/item?id=43166362 )

That said, I would be _very_ interested in an editor/display tool which would show the "liveness" (lifespan?) of a variable.

This is probably a bit of a cheap dismissal.... but I think this article misses the forest for the trees. They set a standard which they manage to meet - but do not really introspect on whether the standard is appropriate or not.

All of these metrics (except variable liveness) are on a method/function level. Guess what this encourages? Splitting everything into three-line methods which makes the codebase a massive pile of lasagna full of global or shared state.

If a method is long to read from top to bottom, the answer isn't always splitting it into 5 smaller ones, sometimes life just has inherent complexity.

    def is_something_valid(something, foo, bar):
        return something > 0 and something != foo and something > bar

    if is_something_valid(something, foo, bar):

    gt_zero = something > 0 
    ne_foo something != foo
    gt_bar something > bar

    if gt_zero and ne_foo and gt_bar:

    something_is_valid = something > 0 and something != foo and something > bar

    if something_is_valid:
        # do stuff

    x = 1 // set variable x equal to 1

    gt_zero = space > 0     // there is some space I guess?

    space_for_logfile = space > 0   // oh, logfiles need space > 20 there's the mistake.

  something_is_valid = something > 0 and something != foo and something > bar
  if something_is_valid:
    # ....

    ...
    #ifndef foo
    break;
    case SomeCondition:
       doSomething().
    #endif
       moreCode();
       break;

I would like to add something to the point made here:

"For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using a well-named variable or helper function can go a long way in reducing the cognitive load for readers. [my emphasis]

  // which is easier and faster to read?
  function funcA(graph) {
    return graph.nodes(`node[name = ${name}]`)
      .connected()
      .nodes()
      .not('.hidden')
      .data('name');
  }

  // or:
  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 names of the functions being called are rather generic, which is appropriate and unavoidable, given that the functions they compute are themselves rather generic. By assigning their returned values to consts, we are giving ourselves the opportunity to label these computations with a hint to their specific purpose in this particular code.

In general, I'm not a fan of the notion that code can always be self-documenting, but here is a case where one version is capable of being more self-documenting than the other.

    function funcA(graph) {
        return (graph
          .nodes(`node[name = ${name}]`)  // message recipient
          .connected()  // recipient friends
          .nodes()
          .not('.hidden')  // inside current scroll area
          .data('name')
          );
      }

This came up at work the other day re: client-side code readability.

In one camp, folks were in favor of component extraction/isolation with self-documenting tests wherever possible.

In the other camp, folks argued that drilling into a multi-file component tree makes the system harder to understand. They favor "locality of behavior".

Our consensus - there needs to be a balance.

Shoutout to the pipe operator in R. The code equivalent of "and then." It helps to unnest functions and put each action on one line. I know R is more for stats and data, but I just think it's neat.

> These metrics definitely are debatable (they were made in the 70’s…)

What is it about a decade that makes contributions produced thereabout "debatable"?

Rust.

I just wanted to point out that "Cognitive Complexity" [1] was not invented by SonarSource, it is an academic principle created in the 1950s and has more to do with psychology than computer science. Computer science has over-simplified the term to mean "hey there's a lot of stuff to remember this is hard".

Psychology tends to have a wider scope of thought and research put into it [2] [3]. For example, one way it's used is not to measure how complex something is, but how capable one particular person is at understanding complex things, versus a different human [4]. This can affect everything from leadership decisions [5] to belief in climate change [6].

I point this out because all too often Engineers hyper-focus on technical details and forget to step back and consider a wider array of factors and impacts - which, ironically, is what cognitive complexity is all about. It's the ability of a person to think about more things in a deeper way. Basically, cognitive complexity is a way to talk about not just things, but people.

We also have a tendency as Engineers to try to treat everyone and everything as a blob. We have to design our language in X way, because all people supposedly work in the same way, or think the same way. Or we have to manage our code in a certain way, because all the team members are assumed to work better that way (usually in whatever way is either easier or simpler).

One thing I wish people would take away from this, is that not only is cognitive complexity actually useful (it describes how language is able to work at all), but some people are better at it than others. So "avoiding cognitive complexity" is, in many ways, a bad thing. It's like avoiding using language to convey ideas. Language and communication is hard, but you're reading this right now, aren't you? Would you rather a pictogram?

[1] https://en.wikipedia.org/wiki/Cognitive_complexity [2] https://www.jstor.org/stable/2785779 [3] https://pubmed.ncbi.nlm.nih.gov/11014712/ [4] https://testing123.education.mn.gov/cs/groups/educ/documents... [5] https://deepblue.lib.umich.edu/handle/2027.42/128994 [6] https://www.sciencedirect.com/science/article/abs/pii/S02724...

There's a cool plugin for vscode called Highlight[1] that lets you set custom regexes to apply different colors to your code. I think a common use of this is to make //TODO comments yellow, but I use it to de-emphasize logs, which add a lot of visual noise because I put them EVERYWHERE. The library I maintain uses logs that look like:

    this.logger?.info('Some logs here');

So I apply 0.4 opacity to it so that it kind of fades into the background. It's still visible, but at a glance, the actual business logic code pops out at you. This is my configuration for anyone who wants to modify it:

    //In vscode settings.json:
    "highlight.regexes": {
        "((?:this\\.)?(?:_)?logger(?:\\?)?.(debug|error|info|warn)[^\\)]*\\)\\;)": {
            "regexFlags": "gmi",
            "decorations": [{
                "opacity": "0.4"
            }]
        }
    },

---

[1] https://marketplace.visualstudio.com/items?itemName=fabiospa...

Interesting how important mere opinion seems to be, because the author doesn't seem to mention two issues that matter a great deal to me:

- Alignment of braces and brackets, instead of an opening brace at the end of one line and the closing brace at the beginning of a subsequent line. - everything I need to see is within an eyespan, instead having to jump to several different files to trace code.

You really should try to pack an unbroken thought as a single line of code as much as possible. That’s the idea behind chaining multiple functions together on one line instead of spreading it out over several lines. Eyes go horizontally more naturally than up and down, it fits our vision’s natural aspect ratio. And stop making deep nestings.

Making a single function call per line assigning output to a variable each time is really just for noobs who don’t have great code comprehension skills and appreciate the pause to have a chance to think. If the variable’s purpose for existence is just to get passed on to a next function immediately, it shouldn’t exist at all. Learn to lay pipe.