The personal website and blog of Ryan Marcus, a graduate student at Brandeis University. Previously worked at HPE Vertica and Los Alamos National Laboratory.
      
    ____                       __  ___                          
   / __ \__  ______ _____     /  |/  /___ _____________  _______
  / /_/ / / / / __ `/ __ \   / /|_/ / __ `/ ___/ ___/ / / / ___/
 / _, _/ /_/ / /_/ / / / /  / /  / / /_/ / /  / /__/ /_/ (__  ) 
/_/ |_|\__, /\__,_/_/ /_/  /_/  /_/\__,_/_/   \___/\__,_/____/  
      /____/                                                    
        
   ___                   __  ___                    
  / _ \__ _____ ____    /  |/  /__ ___________ _____
 / , _/ // / _ `/ _ \  / /|_/ / _ `/ __/ __/ // (_-<
/_/|_|\_, /\_,_/_//_/ /_/  /_/\_,_/_/  \__/\_,_/___/
     /___/                                          
        
   ___  __  ___                    
  / _ \/  |/  /__ ___________ _____
 / , _/ /|_/ / _ `/ __/ __/ // (_-<
/_/|_/_/  /_/\_,_/_/  \__/\_,_/___/                                   
        

Good comment, bad comment

Having spent five years heavily involved in teaching the 2nd semester programming course at Brandeis, I've found myself repeating the same maxim over and over again:

Code is an essay written for two audiences: the computer, and your fellow programmers.

Thinking about the dual-audience nature of code immediately highlights several important aspects of "good code":

  1. When "read" by the computer, good code leads to a correct, effective, and efficient behavior. The desired outcome is achieved (correct) in a reasonable amount of time (effective) without consuming an unreasonable amount of resources (efficient).
  2. When read by your fellow programmers, good code reads like an essay that intuitively, succinctly, and precisely describes the semantics of the problem and how it is solved. In other words, the code is not overly long and verbose (succinct), the code conveys the exact details of what is happening to the reader (precise), and the code does so in a way that makes sense to the reader quickly (intuitive).

In my experience reading thousands (really!) of different solutions to the same problem, I find that the best code follows all six of these principles. In this post, I want to go through each principle, showing both good and bad examples. Each example will be a "real" piece of code written by a student (anonymized, of course), including myself (I have to get the bad examples from somewhere).

For the computer

All in all, contemporary computer science departments do a pretty good job of teaching students to write correct, effective, and efficient code. In fact, new programmers tend to optimize for their robotic audiences over their fellow humans -- "if it works, who cares?" If a computer wasn't part of our audience, we might not choose to write code at all.

However, new programmers are often very frustrated by a computer "not doing what I meant." Understanding how to translate our thoughts into code is a fundamental skill that looks a whole lot like the writing process. The next three sections explain what I believe to be the three properties of good code from the point of view of the computer audience.

Correct

When thinking about the computer as the audience for our code, we want our code to cause the computer to compute the correct result. Correctness may seem like the most obvious principle we would want good code to embody -- but correctness is often subtle, underspecified, or misunderstood. For example, consider the following prompt and solution:

Write a method randItem(int[] arr) that returns an item selected uniformally at random from the the passed array.

public static int randItem(int[] arr) {
    return arr[(int) (Math.random() * arr.length)];
}

At first glance, you might think this code is just fine -- Math.random() returns a uniform value from 0 to 1, multiplying that by the array length and truncating to an int should give a uniformally distributed array index. You might raise a complaint that the programming didn't use Java's Random class, but that's not a correctness issue, right?

It turns out the above code is very subtly incorrect. To understand why, let's simplify the problem a little bit. Imagine that our input array arr is of size 5, and that Math.random() returned a 4-bit random number between 0 and 1. Let's look at the possible values of our array index:

BinaryDecimalArray Index
00000.00000
00010.06250
00100.12500
00110.18750
01000.25001
01010.31251
01100.37501
01110.43752
10000.50002
10010.56252
10100.62503
10110.68753
11000.75003
11010.81254
11100.87504
11110.93754

Spot the problem? In this case, four values of Math.random() map to index 0, but only three values map to all the other elements! That means this method will return the first element of the array with probabilty , while the other elements will each be returned with a probability of ! That's not uniform at all.

Now, Math.random() returns a double, which has a lot more than 4 bits, and only a subset of all possible double values are within the range (0, 1). But, saying there are exactly possible return values for Math.random() and elements in our array arr, this method only potentially works if divides , i.e. n % k == 0.

A more correct way to write this code would be:

private static final Random r = new Random();
public static int randItem(int[] arr) {
    return arr[r.nextInt(arr.length)];
}

Since the nextInt method ensures that we are drawing from a uniform distribution,1 this code produces a result more in-line with the prompt. The point here is that correctness can be subtle, and that a computer will happily do whatever you tell it -- and sometimes the wrong thing can look a whole lot like the right thing at first glance.

Effective

In addition to being correct, we also want our code to cause our computer audiences to compute results effectively. We all know we want code that runs fast, and we learn tools like big-O analysis to help us make good decisions along the way. But sometimes, something seemingly-mundane, or that you think should be fast, can end up taking a very long time. There's even a Tumblr about this phenomenon in very real code. Here's an example of this in code written by myself a long time ago, and by numerous other students ever since:

Write a method combine(String[] args) that returns a String containing each element of args connected with spaces, in order. If args is empty, return the empty string.

public static String combine(String[] args) {
    if (args.length == 0) return "";

    String collector = "";
    for (String el : args) {
        collector += el + " ";
    }

    // Cut off the last space.
    return collector.substring(0, collector.length() - 1);
}

Seems straight-forward enough, right? Cutting off that last space was a little tricky. New developers might not even think to analyze the runtime of this method, or if they did, they might declare it to be linear in the size of args. More seasoned (read: cynical) developers might begin to smell trouble, though.

The way Java represents strings is different from many other languages. In Java, strings are immutable. Once they are created, they can't be modified. So if they can't be modified, what happens when the program executes collector += el + " "? Well, unfortunately, Java will create a brand new string, copying the contents of collector, el, and " " into it. Java will perform this create and copy operation at every loop iteration, which means that our runtime is actually quadratic in the size of args!

Since the good folks working on Java know that developers sometimes need to concatenate strings, Java has a handy StringBuilder class for this exact situation!

public static String combine(String[] args) {
    if (args.length == 0) return "";

    StringBuilder collector = new StringBuilder();
    for (String el : args) {
        collector.append(el);
        collector.append(" ");
    }

    // Cut off the last space.
    return collector.substring(0, collector.length() - 1);
}

Now our code is much more effective.2 The point here is to show that performance bugs can often be hard to notice, and can creep into code in subtle ways that might elude someone who isn't looking closely for them.

Efficient

When I was a new programmer, I didn't really think about the resource usage of my application. It either ran, or it didn't. Most of the programs I was writing were "short-lived", i.e. I started them and waited for them to finish, normally a few moments later. Even if one of my programs used a ton of memory or temporary disk space, I wouldn't notice, because my operating system would liberate all these resources from my greedy program after my program terminated.

However, as I gained more experience and started writing longer-living programs, like servers (or "services", as the kids call 'em nowadays), I started to feel the pain of all my inefficient code. This pain came mostly in the form of memory leaks: bugs in my programs that caused more and more memory to be used up until there was none left.

Many programmers who swear by languages like Java or Python believe that their language's garbage collector -- the part of the language responsible for reclaiming objects we aren't using anymore -- makes them immune to memory leaks. While it is true that these garbage collectors prevent the majority of memory leaks, I was a poor enough Java programmer to create several! Here's an example of one written for a class (not by me, but making the same mistake I did):

Write a class BoundedStack<T> with methods push and pop that behaves like a stack with a bound on the number of items it can contain. Take the bound as a parameter in the constructor.

public class BoundedStack<T> {
    private Object[] storage;
    private int topIdx = 0;

    public BoundedStack(int bound) {
        storage = new Object[bound];
    }

    public void push(T arg) {
        if (topIdx >= storage.length) 
            throw new IllegalStateException();
        storage[topIdx++] = arg;
    }

    public T pop() {
        if (topIdx <= 0) 
            throw new IllegalStateException();
        return (T) storage[--topIdx];
    }
}

Overall, looks fine, right? But this implementation of a bounded stack actually has a memory leak. Consider the following client code:

public static void main(String[] args) {
    BoundedStack<ReallyBigType> s = new BoundedStack<>(5);

    s.push(new ReallyBigType(1));
    s.push(new ReallyBigType(2));
    s.push(new ReallyBigType(3));
    s.pop(); s.pop(); s.pop();

    performTask();
}

Imagine that ReallyBigType is a class that uses a whole bunch of memory. When performTask is called, the programmer might be expecting that all the ReallyBigType instances created beforehand will be freed up, so that performTask can use as much memory as it needs. However, this isn't really the case -- all three instances of ReallyBigType are guaranteed to still be around when performTask is called!

The reason all three instances of ReallyBigType are still around is that our BoundedStack, s, is still holding a reference to them. Even though we decremented the topIdx variable whenever pop() was called, the values in storage (in positions 0, 1, and 2) still point to our instances of ReallyBigType! That's a memory leak. If this were a large, long-running application, small errors like that could build up, resulting in a system that needs to be frequently rebooted.

Luckily, this memory leak is easy enough to fix:

public class BoundedStack<T> {
    private Object[] storage;
    private int topIdx = 0;

    public BoundedStack(int bound) {
        storage = new Object[bound];
    }

    public void push(T arg) {
        if (topIdx >= storage.length) 
            throw new IllegalStateException();
        storage[topIdx++] = arg;
    }

    public T pop() {
        if (topIdx <= 0) 
            throw new IllegalStateException();

        T tmp = (T) storage[topIdx];
        storage[topIdx] = null;
        topIdx--;

        return tmp;
    }
}

By setting the now-unused cells of the storage array to null, we avoid holding a reference, allowing the Java garbage collector to do its work. The point here is that just writing fast, correct code isn't enough: good code behaves well even in long running, resource-scarce environments.

For your fellow programmers

Despite all the difficulties of writing to your computer audience, writing to your human audience is often much harder. After all, writing and composition3 have entire classes of their own! Good code shares a lot of properties with good writing. Specifically, I've found that most good (technical) writing and good code I've read follows three principles. First, I'll give an overview of each of these principles, and the proceeding subsections will expand on them.

Now, luckily for us, we have a bit of a "cheat code" when communicating with human audiences through our code. Specifically, we have comments, which are invisible to our robot audiences, and variable names, which are meaningless to our robot audiences. We can take code the computer is already happy with (correct, effective, efficient), and we can use these two tools to annotate that code to make our human audience happy as well.

As a warning, this part is going to be a lot more subjective than the previous part. I'll do my best to give examples that I think most of the professional software engineers I've worked with would agree on. It's important to realize that a lot of what makes code readable (and what makes writing good or bad) depends on your audience. So if your audience has certain norms -- like indention style, variable naming style, etc. -- following them is in your best interest. Otherwise, you'll be written off from the get-go.

Take a quick look at the two papers below.

Two papers, one poorly formatted

You don't have to be able to see the words to know which paper "looks" better. We have established conventions for writing, and when they aren't followed, it is immediately jarring. And while eschewing these structures can sometimes be considered "art", you generally want to match the expectations of your audience. Software engineers are no different -- if your code doesn't "look" right (perhaps it isn't indented properly, or your variable names are unconventional), the actual code you wrote probably won't matter... the engineer will just dismiss it out of hand.

Part of becoming a good programmer is learning and practicing those norms. You can take advantage of them to convey information quickly and succinctly, or you can fight an uphill battle against them. Unless you've got a really good reason, don't try to make your readers learn a new convention and your complex idea at the same time! Instead, try to use the conventions as rails for your code.

These next sections will continue with a mix of Java and Python, but I'll do my best to include only the norms that are common among most languages. Any specific language you use likely has its own norms (for example, JavaDoc), and you should learn them -- but we'll keep it general here.

Intuitive

Most (scientific / technical) writing has the goal of conveying intuition. The writer has an idea in their head that they've thought about deeply. They've analyzed it, broken it down, tried a million different things, and, at long last, they have a really good understanding of it. The writer's goal is to create a paper that, when read by other scientists, "fast-forwards" them through that potentially multi-year process, transferring the intuition in their head to paper. In this way, reading a paper can be a lot like time travel: it lets you learn what took years to discover in just a few short hours. This is what Newton meant when he said "if I have seen further it is by standing on the shoulders of Giants." Good code works the same way.

This is also an extremely difficult task, and the people who are the best at it often become professors at research institutions. It's a high bar for writing, and it's an even higher bar for code, but the same idea applies. Developers who write code that can convey intuition to other developers drastically boost the productivity of their teams, and ensure the longevity of their projects. The people who are best at it become the director of AI for Google or senior technical fellows at Microsoft.

Consider the following prompt and solution:

Write a method isUnique which, when given a Map<String, String>, returns true if no two keys map to the same value, and false otherwise. Return true if the map is empty.

public static boolean isUnique(Map<String, String> arg) {
    return (new HashSet<>(arg.values())).size() == arg.size();
}

Now, this code is correct, and is reasonably efficient and effective. It is, however, still very bad code. The reason why it is bad code is because it isn't intuitive -- there's no connection from the line as written to the prompt. Here's a way to improve it:

public static boolean isUnique(Map<String, String> arg) {
    // By placing all the map's values into a set, we 
    // collapse any duplicates. Then, if the number
    // of uniqueValues is equal to the total number 
    // of values, all values must be unique.
    Set<String> uniqueValues = new HashSet<>(args.values());
    return uniqueValues.size() == arg.size();
}

In my opinion, this comment and the new, named intermediate variable greatly clarify the code. Of course, it still needs a "top level" comment above the method signature to describe what it does, but that will look a lot like the prompt. Here's another way to improve the original code:

public static boolean isUnique(Map<String, String> arg) {
    // Test if bijection using incl/excl.
    Set<String> AuB = new HashSet<>(args.values());
    return AuB.size() == arg.size();
}

Now, is this "good code"? Maybe if you were writing a mathematics library and all of your potential readers were extremely well-acquainted with the inclusion-exclusion principle, but in most situations this might be even worse than where we started. The majority of software engineers would scoff at this, likely assuming it was pretentious.

Another way to make code more intuitive is to connect each variable, method, etc. to a semantic concept in the real world. Remember, most code is trying to model or represent some real-world thing or idea. For example, consider the following code:

def compute_point(a, b, c):
  a %= 12
  a /= 12
  a += (b / 60) + (c / 60*60)
  a *= math.pi * 2
  return (math.cos(a), math.sin(a))

Now, what does this code do? Well, I know. Because I wrote it. Or I should say, I know today, and maybe next week, but I might not know in a year. However, by just changing the variable names, we can convey a whole lot more intuition:

def compute_hour_hand_position(hrs, mins, secs):
  theta = hrs % 12
  theta /= 12
  theta += (mins / 60) + (secs / 60*60)
  theta *= math.pi * 2
  return (math.cos(theta), math.sin(theta))

Now, the resulting code isn't perfect, but its way better. I can now definitely tell, for example, that this has something to do with time. It probably has something to do with an analog clock. If I have some more context from the file, I might know immediately that this code is computing the (x, y) coordinates for an hour hand on clock. With just a few comments, we can make this obvious:

def compute_hour_hand_position(hrs, mins, secs):
  theta = hrs % 12
  theta /= 12
  theta += (mins / 60) + (secs / 60*60)
  theta *= math.pi * 2 # convert to radians

  # Compute x and y coordinates from the angle.
  return (math.cos(theta), math.sin(theta))

By connecting the code to the real world, I now understand why the first line includes a hrs % 12 (the clock wraps around), and I understand why mins is divided by 60, and why secs is divided by 60*60. Vastly improved over the initial version.

While none of the examples shown here are large or overly-complex, conveying the intuition behind your code only becomes more and more important as you "scale up," i.e. get bigger and more complex. Practicing these techniques on small projects or assignments is critical to slowly developing the skills you need to write more complex code in an intuitive way.

Making code intuitive is easily the hardest task facing any software engineer. It's so extremely difficult that some developers5 have disavowed it entirely, claiming that their code "just makes sense to smarter people." There's a ton of this fake-flex, over-confident compensation in computer science. Try to assure yourself that they're deluding themselves, and then try to avoid them as best you can.

Succinct

One of the biggest mistakes I see new programmers make is being overly verbose with comments. Here's a real(!) example of a simple program that prompts the user for their name and then greets them:

// the main entry method for the program, which
// the java command line program will execute.
public static void main(String[] args) {
    // prompt the user for their name using System.out, which 
    // is a PrintStream class. The PrintStream class has a
    // method called println, which will output the text
    // passed to the console (so that the user can see it) 
    // and then print a newline.
    System.out.println("Welcome to my program! What is your name? ");

    // this makes a new scanner, which can read from 
    // STDIN, located at System.in. The scanner lets us look
    // for tokens, aka stuff the user has entered.
    Scanner sc = new Scanner(System.in);

    // this will create a new string variable called "name" 
    // and set the value to whatever the user types next
    String name = sc.nextLine();

    // finally we use println again, and string 
    // concatenation, with the + operator, to 
    // output or (sic) message!!
    System.out.println("Hello " + name + " ! ");
}

Now, clearly this was written by a new programmer who has paid attention in class, and was told to explain things in comments. To a new developer, all these language features (like the main method) are new, and seem worth... well, commenting on. No points against the new programmer. Part of the learning process.

However, this is a great example of over-commenting. Any software engineer reading this will be quite annoyed wading through the long, verbose comments that are explaining Java language features. As a general rule, you should never use comments to describe a language feature: just assume your reader knows the language as well (perhaps an exception could be made if you use a really weird feature or something). Try to use comments to talk about the why, rather than the what. For example, we could rewrite this program like so:

public static void main(String[] args) {
    // Prompt the user for their name and greet them.
    System.out.println("Welcome to my program! What is your name? ");

    Scanner sc = new Scanner(System.in);
    String name = sc.nextLine();

    System.out.println("Hello " + name + " ! ");
}

We use a single comment to say "the why" of what we are doing, i.e. we are describing the semantics of the code as opposed to the syntax of the code. Then we use some line breaks ("paragraphing") to separate out the input and output sections. Much easier to read.

Verbosity can come from more than just comments. Often, the code itself can be repetitive or overly verbose. Consider the following program:

Write a method checkNames that takes in a List<String> of names and returns the valid names. A name is valid if it begins with a alphabetical character and has a length of at least 3.

public static List<String> checkNames(List<String> names) {
  // creating a new arraylist
  List<String> goodNames = new ArrayList<String>();
  //looping over the input
  for (int i = 0; i < names.length; i++) {
    String currentName = names.get(0);
    if (currentName.length() > 2) {
      if (currentName.charAt(0) >= 'a') {
        if (currentName.charAt(0) <= 'Z') {
          if (currentName.length() > 2) {
            goodNames.add(currentName);
          }
        } else {
          continue;
        }
      } else {
        continue;
      }
    } else {
      continue;
    }
  }
  return goodNames;
}

There's a lot going on here, so we'll try to break it down. The first thing almost any programmer will notice is that the code is very "crowded" -- there's no "paragraphing" or blank lines to indicate the logical chunks of the code. The second .length() > 2 check is also completely redundant. The only two comments are also entirely useless. So let's fix those things first:

public static List<String> checkNames(List<String> names) {
  List<String> goodNames = new ArrayList<String>();

  for (int i = 0; i < names.length; i++) {
    String currentName = names.get(0);

    if (currentName.length() > 2) {
      if (currentName.charAt(0) >= 'a') {
        if (currentName.charAt(0) <= 'Z') {
          goodNames.add(currentName);
        } else {
          continue;
        }
      } else {
        continue;
      }
    } else {
      continue;
    }
  }

  return goodNames;
}

Already, the code is starting to look a little bit more like code. The next thing we notice is that all of those else blocks are useless -- the loop continues without the explicit continue statement anyway. So we can remove them.

public static List<String> checkNames(List<String> names) {
  List<String> goodNames = new ArrayList<String>();

  for (int i = 0; i < names.length; i++) {
    String currentName = names.get(0);

    if (currentName.length() > 2) {
      if (currentName.charAt(0) >= 'a') {
        if (currentName.charAt(0) <= 'Z') {
          goodNames.add(currentName);
        }
      }
    }
  }

  return goodNames;
}

Much shorter, but we can go a bit further. The index variable of the for loop, and the currentName variable, can be replaced with the "for each" pattern.

public static List<String> checkNames(List<String> names) {
  List<String> goodNames = new ArrayList<String>();

  for (String currentName : names) {
    if (currentName.length() > 2) {
      if (currentName.charAt(0) >= 'a') {
        if (currentName.charAt(0) <= 'Z') {
          goodNames.add(currentName);
        }
      }
    }
  }

  return goodNames;
}

Now we're getting somewhere. The next refactor is a bit of a matter of opinion, with older software developers generally siding "against" and younger software developers generally siding "for". One heuristic you can use to try to tell if your code is too verbose is to count the number of levels of indentation. If you're over three, something has probably gone wrong. We can fix it like this:

public static List<String> checkNames(List<String> names) {
  List<String> goodNames = new ArrayList<String>();

  for (String currentName : names) {
    if (currentName.length() <= 2) continue;
    if (currentName.charAt(0) < 'a') continue;
    if (currentName.charAt(0) > 'Z') continue;

    goodNames.add(currentName);
  }

  return goodNames;
}

Every level of indentation adds another level of "context" that the developer has to remember, e.g. "ok, I'm inside this for loop, and this if statement, and this if statement..." By inverting each condition, each if statement now reads more like "ignore names that fail this test." Putting three of them in a row using the exact same formatting takes advantage of parallel structure, just like in writing. By the time you've read through the three if statements, its clear you're left with a name that meets the conditions.

Finally, we can use a little Java API knowledge to make it slightly better, and add some semantic comments, to finish it off:

public static List<String> checkNames(List<String> names) {
  List<String> goodNames = new ArrayList<String>();

  // Find the names that: (1) have three or more characters
  // and (2) start with a letter.
  for (String currentName : names) {
    if (currentName.length() <= 2) continue;
    if (!Character.isLetter(currentName.charAt(0))) continue;

    goodNames.add(currentName);
  }

  return goodNames;
}

If we want to get a little advanced, we could've written the whole thing in an even more succinct way using Java streams:

public static List<String> checkNames(List<String> names) {
  // Find the names that: (1) have three or more characters
  // and (2) start with a letter.
  return names.stream()
          .filter(name -> name.length() > 2)
          .filter(name -> Character.isLetter(name.charAt(0)))
          .collect(Collectors.toList());
}

The point of writing succinct code isn't to show off. In fact, sometimes shorter code can be much more complex and harder to read. For example, consider swapping two variables in Java:

int a = 5;
int b = 9;

int tmp = a;
a = b;
b = a;

Every developer will recognize this as "swap code." It's a common pattern we see everywhere. However, an overly-clever developer might write:

int a = 5;
int b = 9;

a = a ^ b;
b = b ^ a;
a = a ^ b;

"It's shorter by a whole four characters!", proudly declares a young developer who is about to get a talking-to. This is known as the XOR swap algorithm, and it is a neat trick, but its very opaque. Nobody knows what it is doing the first time they see it, and even after they know the trick, they might not be sure everything is in exactly the right order.6 This makes the code very unintuitive. Making code less succinct doesn't conflict with making code intuitive as often as you'd think. Normally, making things shorter, while staying inside the conventions and norms of your language and community, makes things better. Normally. Experience will teach you when something falls into the "XOR swap" category or not.

Overall, making things short and succinct makes your code more readable. Other developer's eyes won't instantly glaze over as soon as they see your code. Since anything but a superhuman-level software developer can only keep so many lines of code in their head a time, making code shorter can often decrease the cognitive load required to read and understand your code. It's tempting to measure the progress of a project by the number of lines of code you've written, but I promise, senior devs will respect a "small codebase that can do a lot" much more than a large codebase that only does a little.

Precise

The last principle for our human audience is precision. Writing comments that tell the user vaguely what is happening is relatively easy -- but writing a comment the succinctly conveys exactly what a program is doing is much, much harder. Luckily, code has to be precise, so at a minimum, an unlucky developer can simply wade through every line to try and figure out what is going on. But we should have loftier goals.

Consider the following prompt and solution:

Write a method that, given a height from which a ball was dropped in meters, computes the velocity of the ball when it hits the ground.

public static double velocity(double height) {
    // compute the v with the energy formula
    return Math.sqrt(2.0 * 9.8 * height)
}

That's short and sweet, and it might even be good enough if your audience has a physics background. But for us non-physics folks... a little more explanation might be nice. Note that the comment "compute the v with the energy formula" is entirely correct. It describes what the code does, semantically, but it is a little hand-wavy. What energy formula? There's a few. What's 9.8? How exactly did you reach this solution?

A better version might be:

private static final double GRAVITY = 9.8;
public static double velocity(double height) {
    // Compute v using the conservation of energy formula:
    //     mass * gravity * height = (1/2) mass * velocity^2
    // Mass cancels. Solve for velocity:
    //    sqrt(2 * gravity * height) = velocity

    return Math.sqrt(2.0 * GRAVITY * height)
}

Now the code is a bit longer, but it is also a lot more precise -- a reader without a ton of physics background can at least step through the algebra to convince themselves that the code is correct. Extracting 9.8 to a variable also makes the code more precisely match the formula. Again, if your target audience is a bunch of physicists, the commentary is probably not required. Context is king.

There's another, very easy way to add precision to code's explanation, although it is only applicable in a few cases. Check out this chunk of code:

double[][] E = new double[k][n];
int[][] J = new int[k][n];

for (int j = 0; j < n; j++) {
    E[0][j] = err(0, j);
    J[0][j] = 0;
}

for (int i = 1; i < k; i++) {
    E[i][i] = 0; 
    J[i][i] = i - 1; 
    for (int j = i + 1; j < n; j++) {
        E[i][j] = Double.POSITIVE_INFINITY;

        for (int x = j - 1; x > i-1; x--) {
            double e = err(x, j);
            double se = E[i-1][x] + e;
            if (se < E[i][j]) {
                E[i][j] = se;
                J[i][j] = x;
            }

            if (E[i][j] < e) break;
        }
    }       
}

Understand what's going? If so, I'm extremely impressed (and perhaps you need to see the last paragraph of the "Intuitive" section). To me, this is just about as bad as it gets. Short, one letter variable names. Absolutely no commenting. Random, complex manipulation of 2D arrays.

... but what if I told you that this one weird comment would transform this code from terrible to completely acceptable? Ready? Here it is...

// Algorithm for optimal piecewise approximation. 
// Transcribed exactly from:
// Mahlknecht, Dignös, and Gamper,  “A Scalable Dynamic 
// Programming Scheme for the Computation of Optimal 
// k-Segments for Ordered Data.”
// https://doi.org/10.1016/j.is.2016.08.002
double[][] E = new double[k][n];
int[][] J = new int[k][n];

for (int j = 0; j < n; j++) {
    E[0][j] = err(0, j);
    J[0][j] = 0;
}

for (int i = 1; i < k; i++) {
    E[i][i] = 0; 
    J[i][i] = i - 1; 
    for (int j = i + 1; j < n; j++) {
        E[i][j] = Double.POSITIVE_INFINITY;

        for (int x = j - 1; x > i-1; x--) {
            double e = err(x, j);
            double se = E[i-1][x] + e;
            if (se < E[i][j]) {
                E[i][j] = se;
                J[i][j] = x;
            }

            if (E[i][j] < e) break;
        }
    }       
}

This is a (in my opinion) complex dynamic programming algorithm for approximating a function of one variable with k line segments. There's absolutely no reason to re-explain an entire scientific paper in the comments of your code, unless you feel like you can do it way better than the original authors. In terms of describing an algorithm, citing the paper (or textbook, with page number) from which it was transcribed is just about as precise (and succinct!) as you can get. Of course, this situation doesn't come up too often, but when it does, it will be one of the most useful-per-character comments you'll ever write.

Conclusions

My experience is limited, and I'm far from the best software developer in the world, but I have worked at quite a few places, programmed for a reasonably long time, and read a lot of programming assignments! So perhaps you don't want to treat these as steadfast rules, but I do think they are good general principles. Here's the cheat-sheet:

Code is an essay written for two audiences: the computer, and your fellow programmers. It should strive to obey these principles.

  1. Correct: the code should produce the correct output.
  2. Effective: the code should run quickly.
  3. Efficient: the code should minimize resource usage.
  4. Intuitive: the code should convey the intuition behind it.
  5. Succinct: the code should be short.
  6. Precise: the code should allow humans to understand exactly what it is doing.

Of course, it is worth noting that you don't always have to write good code. In fact, most of the time, you won't. If you go over to my GitHub, you'll see most of the code I've written is... bad. Real bad. Like, super bad. These are things to strive for, and things to practice. You won't always be able to write code that is especially pristine. But hopefully, bit by bit, year by year, we can get better and better.


  1. You might be wondering how nextInt manages to draw from a uniform distribution -- one approach is to use rejection sampling. First, pick the largest number m less than Integer.MAX_VALUE that the size of your array, n, still divides. Then, sample a random integer from the range (0, Integer.MAX_VALUE). If the sampled integer is less than m, return m % n. Otherwise, try again. 

  2. We could also use Java's streams, and the joining collector, to make the code much shorter and more obvious to a human, but we're only discussing effectiveness here. 

  3. There's a reason it's called "composition." Writing code and writing papers both require careful compositions of abstractions. In computer science, these abstractions are linked lists, stacks, queues, etc. In "traditional" writing, these are paragraphs, citations, ideas, etc. Computer scientists who dismiss literature and other humanities will find themselves at a huge disadvantage. 

  4. This does not mean you cannot create your own abstractions, or use others. It also does not mean you have to explain how an ArrayList works every time you use one. But someone looking at your code should be able to find those details somewhere (perhaps in your code, perhaps in the Java documentation), if they look for them. 

  5. In my experience, they're almost always white dudes. I've been guilty of this in the past, and I hold no grudge against those who avoid me to this day because of it. 

  6. It turns out the XOR swap is also a tad slower than the "normal" swapping code -- mostly due to compiler optimizations.