Why Beautiful Code Matters

by Richard Taylor : 2019-01-30

I would hope that everyone who has ever written software for a living would agree that this block of code is ugly.

//this class adds numbers
class NumberAdder {
    private final int sum;
 public NumberAdder( int a,int y  ) { sum =y +a;}
public int getSUM() {
return sum;

}}

And yet I have been given code to review that contains all these "mistakes". Thankfully not as concentrated, but there none the less. And usually multiple times, even after pointing them out.

The programmers submitting this code were usually very competent. So why were they delivering ugly code? It had to be that they did not appreciate the value of "beautiful code". Getting the code delivered fast seemed to be trumping other considerations.

I gave a talk on "Coding Consisency" subtitled "Why beautiful code is important". The reaction was quite surprising. "Cool. I wish we were taught this stuff in college." about sums it up.

Here are some of the points from that talk.

1. Code Layout Matters

Organisations do not create coding styles to suppress the creativity of their employees. The style exists to remove distractions that make it harder to understand what someone else has written, or what you wrote yourself some time ago. FOr tHE saMe reaSON that ANY WRITteN m@teria1 not foLLowING tHe uSUaL convENTion5 i5 haRDer tO FOLlow.

Much as you may hate "the wrong style" chosen by the team you are in, if you add code in a different style, then you have made the code as a whole harder to read and therefore harder to maintain. You have done a disservice to all the people who will have to work on that code in the future (most likely including yourself).

As an extreme example of where this can lead I once had to modify a big Perl script that had evolved over several years. The code was so hard to follow that each poor soul tasked with making a change just did their own thing to make it work. Whatever it took; just to get the job done and get away from it. Each block was in a different style, because there was no consistent style to comply with. It was a nightmare. And getting worse.

2. Bugs

Good layout conventions can highlight bugs. For example, look at this declaration:

enum Flags { BF, AF, GF, CF, FF, DF }
You have to look quite hard to spot what is more obvious with this layout:
enum Flags { AF, BF, CF, DF, FF, GF }
And even more obvious with this layout:
enum Flags {
    AF,
    BF,
    CF,
    DF,
    FF,
    GF
}
Namely that there is no "EF" flag. So even though you could say that the order of items in an enum does not matter, simply putting them in alphabetical order gives clear benefits.

Obvious? I'm always surprised and a bit disappointed when people add a new item to the end of a previously alphabetic list. And occassionally someone will add an item at a random position in the middle of an alphabetic list...

Also, look at this:

xDimension = 1; xDimension = 2; zDimension = 3;
Versus this:
xDimension = 1;
xDimension = 2;
zDimension = 3;
Where we are setting "xDimension" twice instead of setting "yDimension".

So the key point is "think about the layout". What best exposes the intended patterns to readers of the code?

3. Readability

The bug visibility above is a product of good readability. In languages where white-space is not significant, you have plenty of scope for aiding readability by good use of spacing.

Aligning similar things helps readability:

public TypeZ descriptiveMethodName(TypeA aValue,
                                   TypeB bValue,
                                   TypeC cValue,
                                   TypeD dValue) {

  return new TypeZ(combine(aValue, bValue),
                   combine(cValue, dValue));
}

You can also use blank lines to group related lines of code. Just as short paragraphs in text are easier to read than large blocks with no breaks. For example:

int x = 5;
int y = 4;

int sum = x + y;
int difference = x - y;

verify(x * x - y * y, sum * difference);

And of course you should choose good names for things. Unfortunately, as Leon Bambrick said, "There are two hard problems in computer science: cache invalidation, naming things and off-by-1 errors."

So naming things will get a whole article later in this series. For now lets just acknowledge that naming is hard. Especially when working in a team that does not have a common first language; and when you have to use a number of external libraries with inconsistent naming schemes.

One thing specifically that I see often is contraction of names that keeps the less useful information. For example:

Shape sqShape = new Square(2);
Shape trShape = new Triangle(3, 4, 5);

And needless redundancy...

@Test
public void testForTriangles()

And ... no, naming really is a whole article. Think hard about naming and talk to your team. Do not be afraid to say in a review "I think this would be better called X". Discuss it. Come to an understanding as a group about what makes a good name for you.

4. Comments

Sometimes code is hard to understand because it does something unexpected. For example, in the "Bugs" section above we noticed that a declaration had a sequence A, B, C, D, F, G and we surmised that there was a missing E.

But what if the E was left out deliberately? This is exactly the place to use a comment.

// There is no EF flag because the Earth is not flat.
enum Flags { AF, BF, CF, DF, FF, GF }

Comments explain why the code is doing something, not what it is doing. The code itself describes exactly what it is doing, there is no need to comment that.

Try to pre-empt questions that a reviewer might have about the code. Look for things that you might think were suspicious in someone else's code and explain why there is not a problem.

a.b.c.d(66);    // none of the intermediates can be NULL

// The last parameter must be NULL or we all die instantly.
setSelfDestruct(ALL, NULL);

If there is more work required on the code, a TODO comment which includes the ticket number for that work is really useful. Because without that a TODO is basically an excuse for putting something off indefinitely.

int WWW_END_TIME = 32111;    // TODO : XYZ-666 is this right?

When it comes to objects, a comment explaining why it exists is an explanation of what the object represents, not how it works.

/**
 * Representation of a complex number (x + iy)
 * where i=sqrt(-1) and x,y are floating point numbers.
 */
public class Complex

Finally, bear in mind that short comments are often extracted by tools to create documention. Going back to the ugly code at the start of this article, the comment looks harmless if redundant in the context of the code. But extract it out into documentation and you get this:

NumberAdder : this class adds numbers
Which looks quite ridiculous and not at all helpful. Compared to
NumberAdder : An immutable sum of two integers
Which describes exactly what it is.

5. IDE vs Review Tool

Sometimes your code looks fine in the IDE but the reviewers comment that the formatting is broken. Most commonly this is when the IDE does something clever with mixtures of TAB characters and spaces. Or there is automatic line wrapping or code folding.

Be kind to the reviewers.

When you create a pull-request always check the layout first yourself.

public int oops() {
    // all these lines
        errors += 3;
  // looked to be
    // indented by the same amount in the IDE
        return errors;
}

It is a shame when you have gone to the trouble of laying out your code properly and then make a bad impression on the reviewers because you do not spot that you have mad combinations of hard TAB characters and spaces that the review tool displays "weirdly".

6. Summary

Beautiful code is important because it is

Always write useful comments that explain why not how

Time spent making your code beautiful is a good investment. Take note of comments about style in reviews. Make comments about style in reviews. Do not accept ugly code.