Saturday, August 8, 2009

Changing Return Values to Exceptions, Java/AspectJ Edition

In the last couple of posts, I've been talking about APIs that communicate errors via return values, and how to map those to exceptions. As we saw most recently, one reasonably effective approach in C++ makes use of macros, but it requires a degree of development discipline, and the resulting code remains somewhat unsatisfactory.

In this post, I will present an approach for Java that mitigates these issues. Specifically, you can neatly map return values to exceptions using AspectJ.

(Note that I am using Eclipse for this work. It's great that AspectJ is now an Eclipse project, because it can be incredibly frustrating to use without solid IDE support.)

I'll start with some Java math utility routines that are roughly equivalent to what I was working with in C++. One difference is that in Java, I don't need a custom sqrt() wrapper because the Math.sqrt() method already uses a return value, Double.NaN, to indicate error.
package com.scottmcmaster365.math;

public class MyMathUtils
{
  public static double radiusFromArea(double area)
  {
    return Math.sqrt(area / 3.14);
  }

  public static double circumferenceFromRadius(double radius)
  {
    return 2 * 3.14 * radius;
  }
}
I want to wire up an aspect that runs when these functions are returning, checks for NaN, and throws an exception. With a bit of understanding of AspectJ, this is quite simple:
package com.scottmcmaster365.aspects;

import com.scottmcmaster365.math.MyMathUtils;

/**
 * This aspect maps double NaN return values to runtime exceptions.
 * (For instructional purposes only.)
 * @author Scott McMaster
 *
 */
public aspect MapNaNResultToExceptionAspect
{
  pointcut mathClass() : within(MyMathUtils);
 
  pointcut mathMethods() : mathClass() && execution(* *(..));
 
  after () returning (double d) : mathMethods()
  {
    if( Double.isNaN(d) )
    {
      throw new RuntimeException("Result is NaN");
    }
  }
}
Hopefully even if you're not familiar with AspectJ, that makes at least a little sense to you. Otherwise, I suggest checking out a good tutorial. But in a nutshell, the pointcuts pick out the methods in the MyMathUtils class, and the after advice grabs the return value and allows us to inject code at compile-time.

With this aspect in effect, the client for finding the circumference given the area of a circle will throw a RuntimeException during the call to radiusFromArea():
final double TESTAREA = -1.0;
double radius = MyMathUtils.radiusFromArea(TESTAREA);
double circumference = MyMathUtils.circumferenceFromRadius(radius);
System.out.println(circumference);
That's exactly what I was after: Clean code that doesn't have to check return values explicitly or via macros. Score one for AspectJ. In fact, by making a very minor change to the pointcuts and advice spec, I can even fire the aspect at the Math.sqrt() call site, which (in a more complicated example) narrow the defect down even further. I'll leave this as an exercise for the reader unless someone comments that they want to see it.

Thursday, August 6, 2009

Changing Return Values to Exceptions, C++ Edition

In my last post, I discussed one reason why exceptions are superior to return values from the perspective of an API client. Despite this, you might still run into APIs (albeit hopefully legacy ones) that heavily use status codes and magic return values to indicate errors, and it may not be within your sphere of influence to change. A friend of mine asked me how to deal with this situation.

Ideally we would like to map error results to exceptions. One approach that works in C++ is to leverage the preprocessor via a "failure-checking" macro.

Consider my example from the last post which uses a couple of simple functions to calculate the circumference of a circle given the area. Here is the original client code:
float area = TEST_AREA;
float radius = radiusFromArea(area);
float circumference = circumferenceFromRadius(radius);
cout << circumference << endl;
Let's say that "radiusFromArea" uses the original version of the "mysqrt" function which returns -1 when it receives invalid input, and let's also stipulate that we cannot change the API for any of these functions. We'd like to present an exception to our callers as soon as we detect a bad value returned from one of the functions. We could do that with some spaghetti-ish code:
float area = TEST_AREA;
float radius = radiusFromArea(area);
if( radius < 0 )
{
  throw runtime_error("cannot be negative");
}
float circumference = circumferenceFromRadius(radius);
if( circumference < 0 )
{
  throw runtime_error("cannot be negative");
}
cout << circumference << endl;
(Ideally we would get the exception from "mysqrt" like I demonstrated last time, but recall that for the purposes of this post, we're pretending that we cannot change that function. And yes, in this contrived situation, we could easily check the negativeness of "area", but that would defeat the instructional value of the example.)

As far as isolating where a negative value might be coming from, that works. But it's annoying and ugly and it takes too much typing. That's why C/C++ programmers traditionally tend to be pretty bad about this type of error-checking. We can clean it up considerably with a macro:
#define IfNegativeThrow(x) { \
    if(x < 0 ) \
      throw runtime_error("cannot be negative"); \
    }
The new client code becomes:
float area = TEST_AREA;
float radius;
IfNegativeThrow(radius = radiusFromArea(area));
float circumference;
IfNegativeThrow(circumference = circumferenceFromRadius(radius));
cout << circumference << endl;
This is functionally equivalent to the immediately-preceding example, but it is more concise and (in my opinion) cleaner. You can potentially improve on it by adding an extra macro parameter for the exception message. Macros similar to IfNegativeThrow() can be found in many industrial-strength C++ codebases. This pattern is especially prevalent in old-school COM programming, where it is often used to deal with the infamous HRESULT.

(Shout-out to anybody who still does COM programming like that on a regular basis. Last time I did, I think Bill Clinton was still President.)

There remain some significant drawbacks here. Doing those assignments inside macros still leaves you with less-than-aesthetic code. You still have to maintain discipline about using the macros. And if you are in a language that doesn't support the preprocessor, things get more difficult and/or annoying. (You may have to implement IfNegativeThrow() as a full-fledged method, for example, and incur the requisite overhead.) In a later post, I'll address these issues.

Tuesday, August 4, 2009

Exceptions Versus Return Values

I'm alarmed by the number of programmers I've been talking to lately -- not just C/C++ programmers, but also Java and C# programmers -- who are inclined to forgo the throwing of exceptions and instead use magic return values and/or status codes to indicate an API failure.

Consequenty I sat down to write a blog post about this. As a general rule, before I write something, I check to see if someone else has done it already, and sure enough, a number of people have taken on this issue competently. So instead of imitating their work, I wanted to point you to the best example, namely Ned Batchelder's.

One slight addition I wanted to make was to point out that Ned's article is primarily focused on the use of return values as status codes. I'm actually more concerned about overloading the return value with a sentinel value that is intended to indicate failure (a practice that Ned covers in the section on "Valuable channels"). In my opinion, this is far more insidious than the correct application of the status code idiom because it can time- and space-shift errors far from where they originate, which can unnecessarily complicate debugging.

Take the following square-root function as an example. Let's use C++.
float mysqrt(float n)
{
  if( n < 0 )
  {
    return -1.0;
  }
  return sqrt(n);
}
Here, -1 is being used as a sentinel value to indicate what any high school math student knows, namely, that you can't take the square root of a negative number. Consider what a client of this function must go through. Say I'm starting with the area of a circle, and I want to calculate the circumference using a couple of convenient helper methods:
float radiusFromArea(float area)
{
  return mysqrt(area / 3.14);
}

float circumferenceFromRadius(float radius)
{
  return 2 * 3.14 * radius;
}
Here is the client code:
float area = TEST_AREA;
float radius = radiusFromArea(area);
float circumference = circumferenceFromRadius(radius);
cout << circumference << endl;
If TEST_AREA is negative, we notice that the circumference is whacked (-6.28) when we are all done. That is, hopefully we notice and don't propagate the bad value forward, but I'll give the author of this code the benefit of the doubt for now. Even so, we still have no idea if the error is in radiusFromArea, circumferenceFromRadius, or some unknown function that one of them calls. Now condider what happens if mysqrt is written to throw an exception rather than return a "magic" error value:
float mysqrt(float n)
{
  if( n < 0 )
  {
    throw invalid_argument("can't take sqrt of negative number");
  }
  return sqrt(n);
}
When this code runs on a negative area for input, you see the error immediately in the form of an unhandled exception, with a stack trace pointing right to where mysqrt is called with the bogus value. This is much preferable for debugging, even in this simplistic example. Hopefully this will help convince you that exceptions are superior to the alternatives.