Clear is better than clever [pdf]

Combinations of if/else can seriously damage readability and maintainability.

I (all the time) face verbose code that pretend to be readable and that requires lots of attention just to find out you are setting a single variable across 10-20 lines.

Example:

  if (cond1)
  {
    myVar = 1;
  }
  else if (cond2)
  {
    if (cond3) 
    {
      myVar = 10;
    } 
    else
    {
      myVar = 100;
    }
    ...
  }
  else
  {
    myVar = defaultValue;
  }

* not obvious (until we read all) that we setup a single variable

* combination of conditions difficult to trace for each given value

* high risk of forgetting cases...

Instead of

  myVar =
    (cond1)             ?   1 :         // comment case blabla
    (cond2) && (cond3)  ?  10 :         // comment case bla
    (cond2) && (!cond3) ? 100 :         // comment case blablabla
    ...
                          defaultValue; // comment default

* operation on a single variable obvious

* match between conditions and values obvious

* high visibility on all the cases

* even more readable with proper vertical alignment

* a comment can be added on each line to document the case

[sorry for the multiple edits: I had a hard time getting { } properly display, they ate up the new lines]

Personally, I find that ternary example you posted to be terrible code. We are not using parchment anymore, there is nothing to be gained by saving a couple of lines.

For better or worse, I operate mainly in the PHP world and the recently added spaceship operator is a solution to this. While I think it's a crazy operator on the one hand, on the other it's much simpler to use. https://www.php.net/manual/en/migration70.new-features.php#m...

  return a <=> b;

The 10(!) lines of code you referenced are actually only 8(!) lines compared to your 1(!) line because you included the function declaration and the final brace in your count. The line you proposed is 34(!!) characters wide. I wonder if your statement will continue to grow past 80(!!!) characters as your operand expressions grow in width? Will you transform it back to multi-line when that happens?

However, I'm not sure why we're exclaiming line counts at all and now, apparently, character counts.

Agreed on the semi-colons though.

This depends on the right-to-left associativity of the ternary operator, so this definitely counts as 'clever'.

If you don't put the same type on each return sides of your ternary, you might be in for a nasty surprise.

Now consider this:

        return a > b ? 1 : a < b ? -1 : 0;

        return a == b ? 0 : a < b ? -1 : 1;

        return b < a ? 1 : a == b ? 0 : -1;

Figuring that out is much more difficult than their 8-line counterpart, at least for me. My eyes have to keep jumping from left to right in order to do the checks, like they would do in a piece of uglyfied javascript.

That said, context is everything: if your one-liner was the single line in a function called `comp` with `a` and `b` as parameters, I would not comment on it in a code review.

However, if this line was part of a bigger function, and if instead of `a` and `b` we had `someLongVariableName` and `someOtherVariableName` I would not approve the pull request until this "oneliner" was changed into something else (possibly a call to the `comp` function).

That's nowhere near clear. I frequently use a single level ternary, but this is an abomination and very easy to make a subtle error in.

YES!! And what do you think the statement

  return a < b ? -1 : a > b ? 1 : 0;

And that's the point here. In some sense, you are writing to the lowest common denominator. You're a clever guy who can optimize their code to minimize line count and terse statements are obvious to you, but your code will be touched and looked at by developers of all levels. Developers that will have to fix your bugs, and extend your code, or refactor it - years after you write it.

The standard for collaborative programming is to write code to be understood, not to show how clever you are, or to minimize character count or to minimize line count.

    if (a == b) {
        return 0;
    } else {
        return a < b ? -1 : 1;
    }

The key to clarity and maintainability for me, though, is in the structure. If the code is structured in manageable blocks with clear description on what each block does and how blocks fit together I care little whether within each function the code is concise or verbose.

As a data point, in larger projects with good structure, I generally saw more use concise code than verbose, which may account for my original "concise is better" bias. Just my 2c.

  return a < b ? -1
      :  a > b ?  1
      :           0;

Your point about "macro-readability" is spot on though. Nobody should care about the readability of an isolated piece of code. We should care about the readability of pieces of functionality and that generally requires understanding of a whole function, or even a whole class/module.

Although I've used a ternary operator or 'if' statement from time to time, in general I would like to model the boundrary conditions themselves and separate condition and action (reduces coupling and enhances the possibility of reporting what is going on).

In your case, the a < b statement is most likely part of the natural ordering of the type of whatever variable a and b are. So we should be much more interested in describing the natural ordering of this type, so we can say things like:

    intList.sort(IntOrdering.reverse())

    intStringList.sort(IntOrdering.compose(StringOrdering))

So, creating this natural ordering needs to be done only once, over all the possible programs that can be expressed with ints. Maybe that one, single, time you'd be writing the actual expression you wrote above. But this lower-level logic will be well hidden by the abstractions used to reason at a higher level about correctness when placed orthogonally with other low-level code.

In other words, every time I write ternary expressions or long if-then-else expressions, I am either writing something low-level, or I need to abstract away.

   return a < b ? -1 : a > b ? 1 : 0;

  return -1 if a < b
  return 1 if a > b
  return 0

  if a == b { return 0 }

  return a > b ? 1 : -1

  if a > b {
    return 1
  } else if a < b {
    return -1
  } else {
    return 0
  }

When the code is mostly calls to well-documented APIs, clarity is achievable (though not guaranteed.) if it is more complicated than that, I do not know that there is any way to achieve clarity other than to structure it, internally, in that style.

   return a < b
          ? -1
          : a > b
            ? 1
            : 0;

   return a < b
          ? -1
          : a > b ? 1 : 0;

   return a < b ? -1
                : a > b ? 1
                        : 0;

   return a < b ? -1
                : a > b ? 1 : 0;

Anyone who has the experience of reading lots of highly trivial functions like those, and gotten the feeling "I've read a lot of code, and I understand what each little bit does, but what is the whole thing trying to do?" That's a symptom of excessive verbosity.

Smalltalkers sometimes solved this through executable comments. There would be some code, with a message to "debug this," and we could just highlight the code and quickly and easily walk through it in the debugger. This demonstrates one of the advantages and disadvantages of Smalltalk: Everything could be embodied in the interaction of relatively small objects, so everything tended to be modular, more easily. The disadvantage comes from the fact that the interaction has to be understood to understand the system as a whole.

My excuse for taking this approach in all languages is accessibility, with accessibility applying to people who can't really code. Whatever the language you can hack a few if statements even if you don't know the language. There are people who are learning and you want to make it 'accessible' for them.

Let the compiler do the work and stay off the bleeding edge features. That includes spaceship operators in PHP. I also think it is worth keeping code a few versions old, so in the case of the spaceship operator, I can do if/then/else things for the compiler to do the work and have my code work on someone else's box that is still on PHP 5.4 because the main software they use doesn't work on PHP 7.3.

Or in Javascript, it is obviously best practice to use the fancy new things but if your web page dies in the console on some Apple iPad running an old version of Safari then you can end up refactoring those best practice iterators into for loops. Invariably these refactoring exercises are hastily done, so you can come a cropper.

Another thing is that if I had to explain my code to someone then the nearer to natural language the better. The example ternary would require me to stop and think for a moment, if it was in boring if statements I could explain it to a not-so-technical person in terms they could understand, even if they could not write it themselves.

  match a.cmp(&b) {
      Ordering::Equal => 0,
      Ordering::Greater => 1,
      Ordering::Less => -1
  }

The clarity comes at the cost of some verbosity, but there's only 4 lines of functional code (not counting the closing brace) here compared to the article's 10. You'd get around the same number if you added line breaks to make the ternary example more readable.

When I've come across - or written - "clever" code in my career, it's always been about making something that normally wouldn't fit into memory - or perform slowly - viable. The examples of this are numerous and still happen plenty today on modern hardware (e.g. games, VMs, NaN tagging). There is nothing wrong with "clever" code so long as it's _clearly_ documented and abstracted away with various types, macros, functions, etc.

Then there's all other code. And what I feel the author wanted to say was that anyone coming along later shouldn't have to think twice about what a given line of code is doing.

If there's a bug, sometimes I'll be lucky enough to have a debugger, sometimes not. But, I shouldn't have to second guess a line of code to determine whether or not the bug is there. In your nested ternary expression, I would be second guessing all over the place; what's the precedence? Hell, whenever I use ternary operators I still force myself to parenthesize the conditional, because while operator precedence in C/C++ is well defined, it's not well known.

Clarity is another reason I prefer type safe languages over dynamic ones for critical code. It allows me to be a little more "expressive" with my code where the reader coming along after-the-fact can be assured that everything works out type-wise.

Having worked on code that people's lives depended on, there are times when getting it wrong isn't an option. I can only feel for the software engineers of the 737 MAX going back over gobs of code and trying to dissect what went wrong and come up with a fix. They'd be heavily scrutinizing every line of code, and I can promise that a nested ternary would be very scrutinized. But, so would a deep chain of if, elsif, elsif, ... and recursive functions (which are often a _big_ no-no in mission-critical code). Although, I don't think anyone here effectively argue that recursion is something to be considered "clever".

It's all about a code reader being able to _prove_ to themselves that the code does _exactly_ what it looks like it does and nothing more.

I'm not a Rust fanboy, but - just like static type checking - the borrow checker adds one more level of surety to the reader that another thread isn't coming along and stomping over the data. And this is without needing the whole context of the code base. This is a _big_ deal.

  return a > b ? 1 : a < b ? -1 : 0;  
  return a < b ? -1 : a == b ? 0 : 1;
  return a == b ? 0 : a < b ? 1 : -1;
  return a > b ? 1 : a == b ? 0 : -1;
  return a != b ? a > b ? -1 : 1 : 0;  
  return a != b ? a < b ? 1 : -1 : 0;  
  

The less lines of code, the faster you can read a code base, apply fixes and have less bugs. Obviously you can take that too far and make code so dense no one can read it. It is finding a balance between the two. Comments can go a long way towards making complex code understandable within reason.

    return a < b ? -1 : a > b;

Or, if that's too many parens for you:

(mcond (< a b) -1 (> a b) 1 0)

The definition of MCOND is four lines of code and can be found here:

https://github.com/rongarret/ergolib/blob/master/core/ergoba...

In the end, what's simpler, spending 15 seconds looking at a one liner using well known language syntax, or digging through half a screen of if states, for loops (use iterators instead), for two examples.

```

(cond ((< a b) 1)

      ((> a b) -1)

      (t 0))

edit 1: formatting

If anything, it should remind you that clarity depends on the context.

It’s concise and simple to understand. Instead of returning 1 or -1 it returns a positive or negative value with I think is equally effective.

I mean, I get that your example is directly related to the point you are making in verbosity, I don’t even necessarily disagree with you, but your example isn’t very clear to me.

Yours is in the style of James Joyce where it's very dense...

the easiest to read is probably somewhere in between

Is this a joke? This comes off like an order of operations test.

Tangentially related for some of the younger devs: The predecessors on your project probably weren't idiots. It's possible, but they were likely working with a different set of requirements under a different scope. Even the code that it morphed into over time was probably a good enough decision for the constraints at the time.

Because they're equivalent. There no uninitialized values in Go; everything not given an explicit initializer is zeroed out.

"So are we writing code to be read by Go experts? Go noobs? Software Engineers with general background in many lanaguages, but perhaps not go? Noob programmers?"

One of the nice things about Go not particularly appreciated by a lot of the HN community (as evidenced by a lot of the other comment threads) is precisely that code targeted to those various targets will not differ that much.

I also program in Perl 5 a lot, unfortunately, and to make it professionally palatable, I do indeed have to program in a dialect of Perl that has been chosen to be something that can be read even if you are not a Perl expert. I tend to avoid dragging in huge rewrites of the object system, for instance, in favor of the built-in support, which I can reasonably expect everyone to know. I avoid using autovivification, etc. I often see code written by a Perl expert in the code base, and have at times even torn it apart and put it back in the LCD dialect we locally use, just so we don't have this sudden, confusing chunk of code. The code is messy for many of the other reasons I've come to dislike large dynamic-language codebases, but you won't encounter $% += $(-/\#/[[]]/l; in our code base.

In Go, there's not much need to cut back to a subset of the language. There's only a handful of constructs you need to avoid, and it's the same code for almost every level. The "programmer who only knows half of Go" isn't that big a deal, because anyone who knows half of Go can finish the rest of it in a couple of hours.

I value this professionally. I understand why people who seek other things from their programming do not, because I also have times I seek those things. My personal codebase is much more mixed. But when I'm being paid to produce code, I'm not being paid to feel like I'm really clever for stringing one line together that downloads a web page, parses it through an HTML parser, and strips out all the text, returning the text nodes as a stream of strings or a concatenated string or whatever. Yes, I can do that, in several languages, but except in certain specialized circumstances, that is low quality professional code, even if it "works", because when I expect someone with two years of experience to make a change to that code base, I'd like them to be able to do it in a reasonable period of time, and with a reasonable understanding of the consequences and tradeoffs, not as a modification of an incantation.

(Every time someone tells some story about the one guy who has been around a long time and knows all the things and is the only one who can make any changes to any production system because it's all his code and only he can understand it because it's all crazy nonsense to anyone else, and all the schedules are busted because even he no longer can keep up with the mess, we all have a good laugh and condemn him. But when we get told that in order to avoid that, we may have to not write the cleverest possible code that we can, and that we need to prefer simple, effective code that works, and is easy to understand, a lot of us get all offended at the infringement of our rights to cleverness. Well... it's the same thing, just two different angles.)

In my experience, loops and recursion are lower level constructs that should ideally be used only in libraries (map, filter, limit, sum, distinct, etc [1]) and in hot code paths (for optimization).

Kevlin Henney makes a good observation on one of his talks about how "you have written all the loops you will never need in your life" [2], and those are constructs such as map/filterand friends, present in LINQ, Java Streams, Ruby Enumerables, etc.

His argument is that code made with those primitive is clearer than code made than procedural loops, because when you read it you don't have to "execute the code in your head".

I'd argue that using procedural loops is what's actually "too clever" here, despite being more verbose!

-

[1] I have mixed feelings about "Reduce". It is hard to use and, most of the time, hard to read as well. It's too low-level.

[2] https://youtu.be/APUCMSPiNh4?t=3317

> However, I've never seen a convincing and objective description of clear code.

I take "clever code" to mean "code that looks pretty on surface the but requires the reader to dig in in order to understand the intent".

The problem with clever code is that it is misleading. It lacks empathy. You assume the next programmer will understand it at first glance just because it looks cool or pretty, but they'll struggle to parse it.

---

> Sometimes people mean that clear code is verbose code.

I also consider some verbose code to be "clever code" too. Most of the time it is just people using hammers where screwdrivers would be more appropriate:

- Unnecessary structures - eg: classes that could be functions taking 2x or 3x more space

- Unnecessary usage of polymorphism - eg: inheritance chain that could be a simple if

- Excessive indirection - eg: layered code where most of the time the layers don't do anything

- Plain wrong abstractions - eg: using query builders to build queries that would be smaller and more readable in SQL

- Fear of making classes too big - eg: instead of adding a method to a class, making a second class that knows too much about the innards of the first one

- Procedural code disguised as OOP - eg: breaking up a method into multiple private ones, but ending up having a lot of instance variables that were local variables before. When everything could be a single function.

---

> The best we have is cyclomatic complexity, but there's some reason to believe that line count may be a better indicator (which can't be good). And cyclomatic complexity completely misses the effect of mutable or immutable state, the presence of bad APIs, poor variable naming, etc.

Great observations. I agree 100%.

Since you mentioned it, I find cyclomatic complexity a bit too easy to game. I always wanted to have a metric that prevented people doing that, and took multiple methods into account.

When you break a method in three or four without adding REAL abstractions (a.k.a. "things you don't have to follow with the debugger to understand"), you're not making it easier to read, in fact you're making it harder because the reader has to jump around your code.

Of course, it's harder for machines to know the difference between good and bad abstractions. But I think we should take that into account in code reviews and such.

I'd say it's objectively prettier most of the time, but unfortunately that's exactly what introduces double negatives or other ambiguous stuff and makes the code more confusing.

It's a nice refactoring step, though: apply it, extract the condition into a well-named variable and test that condition.

Less code does not mean less bugs.

I've heard comedians talk about a set not having any fat in it... every phrase, every short pause, etc. is exactly what is needed to make the bit funny. To stretch this analogy a bit, you can trim so much fat that you start taking the meat and the audience can no longer follow the joke. We need that kind of philosophy in coding.

In C++ it would look like this:

  (1) x = thing.Property;
  (2) x = thing->Property; (or (*thing).Property)

  (1) x = thing.Property
  (2) x = thing.Property

They're semantically the same. The type of thing isn't, but that's syntax.

> Unmarshal parses the JSON-encoded data and stores the result in the value pointed to by v. If v is nil or not a pointer, Unmarshal returns an InvalidUnmarshalError.[1]

In (1) the '&' in '&thing' means roughly "the address of", so you are passing "the address of thing" i.e. a Thing pointer that points to the thing variable.

1: https://golang.org/src/encoding/json/decode.go

I came from JavaScript so it wasn't too hard, but I was writing mostly with C when I first encounter go. With JavaScript I heavily leaned on properly named variables to understand the underlying type within the context of the code I was working on. In go however, short and nondestructive variables are common place, with the lack of types in declarations I have to reply on properly named functions and general context of the code I'm working in.

I'd be curious to see what go looks like with explicit vars for most things.

    Focusing on readability of the code and 
    putting "machine execution" second, 
    means putting the user experience 
    second

More charitably, a better argument is that it is easier to adapt a compiler to reoptimize it than to tweak the entire code base - let alone the costs for the gains.