PUBLIC OBJECT

Seductive Code

Published on

It’s careful work balancing tradeoffs when writing code and designing APIs. Here’s some everyday Java code:

for (Protocol protocol : protocols) {
  if (protocol != Protocol.HTTP_1_0) {
    result.writeByte(protocol.toString().length());
    result.writeUtf8(protocol.toString());
  }
}

On Android—where garbage collector costs aren’t necessarily negligible—I make my code slightly uglier to save an implicit Iterator allocation:

for (int i = 0, size = protocols.size(); i < size; i++) {
  Protocol protocol = protocols.get(i);
  if (protocol != Protocol.HTTP_1_0) {
    result.writeByte(protocol.toString().length());
    result.writeUtf8(protocol.toString());
  }
}

Java 8 invites me to make the opposite tradeoff: to use streams and lambdas like a functional programmer:

protocols.stream()
    .filter(protocol -> protocol != Protocol.HTTP_1_0)
    .forEach(protocol -> {
      result.writeByte(protocol.toString().length());
      result.writeUtf8(protocol.toString());
    });

Here we’ve got three perfectly-reasonable ways to write the same code, and therefore the perfect opportunity to have a disagreement on how this loop should be written.

When a programming language, API, or design pattern is seductive its cosmetic beauty hides its structural limitations. I’m frustrated when making my code read better harms how efficiently it runs or how maintainable it is.

Seductive and Slow

Recently I was doing some Python pair programming. Its array operators are fantastic: you can get the data you want without ceremony. Our task was to append a bunch of values to a stack. It’s something I’d typically write like so:

stack = []
for i in values:
    stack.append(i)

But my pairing partner surprised me by concatenating:

stack = []
for i in values:
    stack += [i]

The concatenating code is mathematically elegant but its performance is atrocious. Each iteration of our loop needlessly copies the entire stack. It’s a performance bomb that is eager to explode.

Seductive and Skimpy

Much of the code in Moshi is coping with failure. We’ve paid close attention to how it behaves when things break. JUnit’s expected exceptions feature make it easy to test failure cases like an int/double type mismatch:

@Test(expected = JsonDataException.class)
public void integerMismatchWithDouble() throws IOException {
  JsonReader reader = newReader("[1.5]");
  reader.beginArray();
  reader.nextInt();
}

This feature is easy to use but I particularly dislike it. My gripe is that its simplicity limits the utility of the test. Here’s a better one:

@Test public void integerMismatchWithDouble() throws IOException {
  JsonReader reader = newReader("[1.5]");
  reader.beginArray();
  try {
    reader.nextInt();
    fail();
  } catch (JsonDataException expected) {
    assertThat(expected).hasMessage("Expected an int but was 1.5 at path $[0]");
  }
  assertThat(reader.nextDouble()).isEqualTo(1.5d);
  reader.endArray();
}

This test is bigger and better:

  • It confirms that the nextInt() call throws the exception. With JUnit’s expected exceptions the test passes if any line throws a JsonDataException.
  • It confirms that the exception has a helpful message.
  • It confirms that the JsonReader is in the expected state after the exception. Calling code can recover from this failure if necessary.

Use of the @Test(expected=...) feature seduces you into writing loose tests because your test ends as soon as the exception is thrown.

Seductive Syntax

I’m enjoying Kotlin. The language is brilliant because you can write code that’s efficient, maintainable, and readable all at the same time. I like that its data classes are simple and capable immutable types without boilerplate. And I like how extension methods let me build new abstractions.

My only complaint is in some syntax. What does this program print?

@Test fun test() {
  val maximumHeightPx = 400
  val textLineHeight = 100

  val maxTextBaseline = (maximumHeightPx - textLineHeight) / 2
      + textLineHeight
  println(maxTextBaseline)
}

To my surprise, the program prints 150 because Kotlin’s parses + textLineHeight as a separate statement and not a continuation of the previous line.

Kotlin’s syntax seductively omits semicolons. That looks good but the cost is too steep! Whitespace is significant in Kotlin and wrapping lines may change their behavior.

Real Beauty

When designing an API be very careful about including seductive features. It’s much nicer when the readable code is also the fastest and most capable.