Magic Numbers, Semantics, and Compiler Errors

Magic numbers are considered such a scourge upon code bases that there tools out there to automatically find them and root them out. After all, who knows what setValue(6); really means? There are also many approaches to giving better names to magic numbers. Not all of them are good, though. For example, we have a callback that sets a completion percentage on a status:

void callback(Status status) {
    status.updateProgress(0, "Starting");
    // Do some stuff...
    status.updateProgress(25, "Some stuff done");
    // Do some more stuff
    status.updateProgress(50, "More stuff done");
    // Reticulating splines
    status.updateProgress(75, "Splines reticulated");
    // Validate everything
    status.updateProgress(100, "Complete");
}

Those percentages would get flagged as magic numbers by the automatic tools. The developer would need to extract those out to constants to make that tool happy. A thoughtful developer would create well-named, class-level constants for each percentage:

private static final int ZERO_PERCENT = 0;
private static final int TWENTY_FIVE_PERCENT = 25;
private static final int FIFTY_PERCENT = 50;
private static final int SEVENTY_FIVE_PERCENT = 75;
private static final int ONE_HUNDRED_PERCENT = 100;

void callback(Status status) {
    status.updateProgress(ZERO_PERCENT, "Starting");
    // Do some stuff...
    status.updateProgress(TWENTY_FIVE_PERCENT, "Some stuff done");
    // Do some more stuff
    status.updateProgress(FIFTY_PERCENT, "More stuff done");
    // Reticulating splines
    status.updateProgress(SEVENTY_FIVE_PERCENT, "Splines reticulated");
    // Validate everything
    status.updateProgress(ONE_HUNDRED_PERCENT, "Complete");
}

A more hapless developer, or even one mass-fixing magic constants, would not take as much care:

private static final int ZERO = 0;
private static final int TWENTY_FIVE = 25;
private static final int FIFTY = 50;
private static final int SEVENTY_FIVE = 75;
private static final int ONE_HUNDRED = 100;

void callback(Status status) {
    status.updateProgress(ZERO, "Starting");
    // Do some stuff...
    status.updateProgress(TWENTY_FIVE, "Some stuff done");
    // Do some more stuff
    status.updateProgress(FIFTY, "More stuff done");
    // Reticulating splines
    status.updateProgress(SEVENTY_FIVE, "Splines reticulated");
    // Validate everything
    status.updateProgress(ONE_HUNDRED, "Complete");
}

We've lost the notion that these are percentages. But that same developer might be even more "helpful" and notice we use those same numbers in other places. They should all be in a shared constants file:

public class AppConstants {
    public static final int ZERO = 0;
    public static final int TWENTY_FIVE = 25;
    public static final int FIFTY = 50;
    public static final int SEVENTY_FIVE = 75;
    public static final int ONE_HUNDRED = 100;
}

void callback(Status status) {
    status.updateProgress(AppConstants.ZERO, "Starting");
    // Do some stuff...
    status.updateProgress(AppConstants.TWENTY_FIVE, "Some stuff done");
    // Do some more stuff
    status.updateProgress(AppConstants.FIFTY, "More stuff done");
    // Reticulating splines
    status.updateProgress(AppConstants.SEVENTY_FIVE, "Splines reticulated");
    // Validate everything
    status.updateProgress(AppConstants.ONE_HUNDRED, "Complete");
}

The automated tools are happy, and we've even made our code more DRY! We could use any one of our application constants, like AppConstants.TWO_HUNDRED. Oh, wait, that isn't a valid percentage. Or we could use AppConstants.LONG_BITS_COUNT. That isn't even trying to be a percentage. That's not good. We've given the numbers names and moved them around, but we haven't improved the semantics of those values. They are just as meaningless as constants as they were as raw values. We may have dug ourselves into a hole. Let's back up and see if we can do something different.

Percentages are a well-defined concept. They are values from 0 to 100, inclusive. They should never be negative, and they should never be greater than 100. Because our code is using int types, we know they shouldn't have fractional components either. These constraints are not all expressed by using an int. We could pass in invalid values outside of the range.

The updateProgress method on Status could have a runtime check on the values. This approach has two shortcomings though. First, we would not know if we gave an invalid value until we ran the code. Second, the updateProgress method now has to do more things. If we have percentages in other places, they also have to do the same checking, repeating code across the code base.

Because percentage is a well-defined concept, we could instead encode it as a type in our system:

public enum Percent {
    // We could define every percent from 0 to 100, but our app only uses these right now.
    ZERO(0),
    TWENTY_FIVE(25),
    FIFTY(50),
    SEVENTY_FIVE(75),
    ONE_HUNDRED(100);

    private final int numericValue;

    Percent(int numericValue) {
        assert numericValue >= 0 && numericValue <= 100;
        this.numericValue = numericValue;
    }

    @Override
    public String toString() {
        return numericValue + "%";
    }
}

We've defined percentage semantically. It is its own thing now. We don't need to use a primitive type masquerading as a percentage. We can trivially add more values. We've put in an assertion on the boundaries of the numeric value we can have. We now don't need to check our bounds everywhere else. The developer will get an error as soon as the enum is loaded if they create a value outside of the boundaries. There will never be an invalid Percent instance. Weaving our type into the code, we would now see:

void callback(Status status) {
    status.updateProgress(Percent.ZERO, "Starting");
    // Do some stuff...
    status.updateProgress(Percent.TWENTY_FIVE, "Some stuff done");
    // Do some more stuff
    status.updateProgress(Percent.FIFTY, "More stuff done");
    // Reticulating splines
    status.updateProgress(Percent.SEVENTY_FIVE, "Splines reticulated");
    // Validate everything
    status.updateProgress(Percent.ONE_HUNDRED, "Complete");
}

We've not only given a better name to our constants, but given ourselves a compile-time check that we aren't using an invalid percentage. We also can't inadvertently pass in a length or a number of seconds where we really need a percentage. Our code now has a greater semantic meaning than it would have with the primitive constants alone. Our code will fail to compile, giving us immediate feedback. This means that our code will be more correct and have fewer errors. It will be easier to maintain. It will be better code.