"Refactoring" a constant-time method into linear time

Recently, I had the (dis-)pleasure to stumble upon a coding horror created by a colleague of mine. When I told Pietro about it, he graciously asked me to write a post about it. So, here we go!

If you know Java and haven’t lived with your head under a rock for the past 7-odd years, you surely know about streams. We all know and love streams, right? Well, what I love even more than streams is applying my judgement and thinking whether it is or isn’t a good idea to use one.

Take this simple and innocent-looking piece of code, for example:

int lastElement(int[] array) {
  if (array.length == 0) {
    throw new RuntimeException("Array is empty");
  }
  return array[array.length - 1];
}

It doesn’t get any simpler than that.

But if you just want to use streams everywhere, you might be tempted to convert it as follows:

int lastElement(int[] array) {
  return Arrays.stream(array)
    .reduce((first, second) -> second)
    .orElseThrow(() -> new RuntimeException("Array is empty"));
}

Spot the difference? You just converted a constant-time array access into a linear-time scan!

This is not necessarily an issue with streams (the same coding horror can be achieved with a good old-fashioned for loop, of course), but it just serves to prove that:

• applying your judgement is better than blindly use the new shiny API
• it is important to always consider the complexity (both in time and space) of your code.

Needless to say, the pull request that contained this change was NOT approved!