Code Style

Magic Number

Definition: An unnamed numeric literal in code (except for -1, 0, 1, and 2)

Example from L16:

public class EnergyPriceService {
    private final EnergyPriceApi api;
    private final Clock clock;

    public EnergyPriceService(
        EnergyPriceApi api, Clock clock) {
        this.api = api;
        this.clock = clock;
    }

    public boolean isOffPeakHours() {
        int hour = LocalTime.now(clock).getHour();
        return hour >= 22 || hour < 6;
    }
}

Poll: What's wrong with magic numbers?

public boolean isOffPeakHours() {
    int hour = LocalTime.now(clock).getHour();
    return hour >= 22 || hour < 6;
}

Text espertus to 22333 if the
URL isn't working for you.

https://pollev.com/espertus

Open-ended poll

Benefits of Named Constants

Readability

static final int SECONDS_PER_MINUTE = 60;
static final int MINUTES_PER_HOUR = 60;
static final int SECONDS_PER_HOUR = 3_600;
static final int HOURS_PER_DAY = 24;
static final int SECONDS_PER_DAY = SECONDS_PER_HOUR * HOURS_PER_DAY;

Safety

startTimer(SECONDS_PER_DAY);
startTimer(86400);

Maintainability

static final int CREDITS_TO_GRADUATE = 128;
static final int NUMBER_OF_CAMPUSES = 14;

• Point out use of underscore in 3_600

Think-Pair-Share: What would be good names?

    private static final int ______________ = 6;
    private static final int ______________ = 22;

    public boolean isOffPeakHours() {
        int hour = LocalTime.now(clock).getHour();
        return hour >= 22 || hour < 6;
    }

I considered making this a word cloud, but PollEV converts START_PEAK_TIME to start peak time.

Notorious Example: Fast inverse square root

 float Q_rsqrt( float number )
    {
        long i;
        float x2, y;
        float threehalfs = 1.5F;

        x2 = number * 0.5F;
        y  = number;
        i  = * ( long * ) &y;  // evil floating point bit level hacking
        i  = 0x5f3760df - ( i >> 1 );               // wtf?
        y  = * ( float * ) &i;
        y  = y * ( threehalfs - ( x2 * y * y ) );   // 1st iteration
    //  y  = y * ( threehalfs - ( x2 * y * y ) );   // 2nd iteration, this can be removed

        return y;
    }

Source: Quake III Arena source code, via Wikipedia

• This calculates the fast inverse square root of a number (1/sqrt(x))
• This was part of the 1999 first-person shooter game, Quake III Arena
• The name "threehalfs" is no better than just using 1.5
• Note that this also has terrible comments, which leads us to our next topic

Poll: What were you taught about in-line comments in previous classes?

Text espertus to 22333 if the
URL isn't working for you.

https://pollev.com/espertus

• Another open-ended poll

Best Practice for Writing Code Comments

• This lecture is based on an article I wrote for the Overflow (Stack Overflow blog).
• I wrote it because I couldn't find any good existing article.

Principles

• "Code tells you how. Comments tell you why." — Jeff Atwood, co-founder of Stack Overflow

• Record your brain state for future you and other readers.

"Unfinished Work" by MonkeyUser, CC BY-NC 4.0

Rule 1: Comments should not duplicate the code

Don't do this:

i = i + 1; // Add one to i

What should you write instead?

Probably nothing, unless the behavior is non-obvious:

// Remove consecutive duplicates from array: [1,1,2,2,2,3] -> [1,2,3]
int writePos = 0;
for (int i = 0; i < arr.length; i++) {
    arr[writePos++] = arr[i];

    // Skip all consecutive duplicates
    while (i + 1 < arr.length && arr[i] == arr[i + 1]) {
        i = i + 1; // Skip duplicate; already wrote this value
    }
}

This is an instance of: Code tells us how; comments tell us why.

Don't Write Cat Comments

Rule 2: Good comments do not excuse unclear code

private static Node getBestChildNode(Node node) {
    Node n; // best child node candidate
    for (Node node: node.getChildren()) {
        // update n if the current state is better
        if (n == null || utility(node) > utility(n)) {
            n = node;
        }
    }
    return n;
}

Instead, rewrite:

private static Node getBestChildNode(Node node) {
    Node bestNode;
    for (Node currentNode : node.getChildren()) {
        if (bestNode == null || utility(currentNode) > utility(bestNode)) {
            bestNode = currentNode;
        }
    }
    return bestNode;
}

This leads us to Rule 3...

Rule 3: If you can't write a clear comment, there may be a problem with the code

	/*
	 * If the new process paused because it was swapped out, set the stack level
   * to the last call to savu(u_ssav).  This means that the return which is
   * executed immediately after the call to aretu actually returns from the
   * last routine which did the savu.
	 *
	 * You are not expected to understand this.
	 */
	if(rp->p_flag&SSWAP) {
		rp->p_flag =& ~SSWAP;
		aretu(u.u_ssav);
	}

Source: Unix source code

Rule 4: Comments should dispel confusion, not cause it

"[Peter Samson] was particularly obscure in refusing to add comments to his source code explaining what he was doing at a given time. One well-distributed program Samson wrote went on for hundreds of assembly-language instructions, with only one comment beside an instruction that contained the number 1750. The comment was RIPJSB, and people racked their brains about its meaning until someone figured out..."

— Steven Levy

Any guesses for the connection between 1750 and RIPJSB?

Source: Hackers: Heroes of the Computer Revolution

Rule 4: Comments should dispel confusion, not cause it

"[Peter Samson] was particularly obscure in refusing to add comments to his source code explaining what he was doing at a given time. One well-distributed program Samson wrote went on for hundreds of assembly-language instructions, with only one comment beside an instruction that contained the number 1750. The comment was RIPJSB, and people racked their brains about its meaning until someone figured out that 1750 was the year Bach died, and that Samson had written an abbreviation for Rest In Peace Johann Sebastian Bach."

— Steven Levy

Source: Hackers: Heroes of the Computer Revolution

Rule 5: Explain unidiomatic code in comments

final Object value = (new JSONTokener(jsonString)).nextValue();
// Note that JSONTokener.nextValue() may return a value equals() to null.
if (value == null || value.equals(null)) {
    return null;
}

Eran Yarkon, who commented on the article, suggested this rewrite:

final Object value = (new JSONTokener(jsonString)).nextValue();
if (value == null || value == JSONObject.NULL) {
    return null;
}

Rule 6: Provide links to the original source of copied code

Why might it be useful to include a link to a StackOverflow post?

Text espertus to 22333 if the
URL isn't working for you.

https://pollev.com/espertus

• Open-ended poll
• The Stack Overflow keyboard was originally an April Fool's Joke but was really sold

Linking to Source Code Provides Readers with...

• what problem was being solved
• who provided the code
• why the solution is recommended
• what commenters thought of it
• whether it still works
• how it can be improved

Example: /** Converts a Drawable to Bitmap. via https://stackoverflow.com/a/46018816/2219998. */

This also honors the open-source license.

• Clicking through lets you see the user is highly-rated.
• A comment provides information on an edge case.

Rule 7: Include links to external references where they will be most helpful

Links to specifications are especially useful.

// http://tools.ietf.org/html/rfc4180 suggests that CSV lines
// should be terminated by CRLF, hence the \r\n.
csvStringBuilder.append("\r\n");

You can get the full details and see if there are any updates.

• Sidebar shows standard has been updated by RFC 7111

Rule 8: Add comments when modifying code

This is especially important when fixing bugs.

  // NOTE: At least in Firefox 2, if the user drags outside of the browser window,
  // mouse-move (and even mouse-down) events will not be received until
  // the user drags back inside the window. A workaround for this issue
  // exists in the implementation for onMouseLeave().
  @Override
  public void onMouseMove(Widget sender, int x, int y) { .. }

Best practice is to include the issue number:

// Use the name as the title if the properties did not include one (issue #1425)

These details help readers understand if code is still needed and how to test it.

Rule 9: Use comments to mark incomplete implementations

Sometimes, it's necessary to commit incomplete code:

// TODO(hal): We are making the decimal separator be a period,
// regardless of the locale of the phone. We need to think about
// how to allow comma as decimal separator, which will require
// updating number parsing and other places that transform numbers
// to strings, such as FormatAsDecimal

Better yet, add an issue to your tracker, and reference the issue in the comment.

Resist the temptation to hide known deficiencies.

[Trivia for instructors: hal is Hal Abelson, who led the App Inventor project.]

Summary

It is well worth taking the time to:

• give good name to constants
• write and update comments

Your colleagues and future you will appreciate it.

Words of wisdom

"Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. Code for readability."

— John F. Woods

https://stackoverflow.com/a/878436/631051

Bonus Slide