Adding Units To Magic Numbers

Question

Considering this question and the most upvoted answer, and his specific example of

public static final int THREE = 3;

might it make sense to allow this sort of usage if we added units to the declaration? I mean like this:

public static final int THREE_MINUTES = 3;

or maybe this:

public static final int THREE_GALLONS = 3;

I'm thinking in terms of stuff I'd flag in a code review. I would definitely flag final int THREE = 3 but does it seem like a generally reasonable exception to allow numbers that add unit of measure?

Answer 1

The issue is not only with the lack of units, but the fact that it is not clear what three of those units represent. Do you only have three minutes to complete a task? Then the constant might be better named as MAXIMUM_TASK_DURATION. Is three gallons the capacity of some container? Then we could use the name CONTAINER_CAPACITY. Your original names only add precision to what the value is, but not how it is intended to be used, which is the crux of the issue.

The lack of units in those suggested constants might also be an issue, albeit a separate one. One possibility would be indeed to add the units in the constant name. Another would be to avoid primitive obsession and use a more appropriate type, such as Duration (which is already provided by the JDK), or Volume (which could be a value object created specifically for the domain of your application).

Answer 2

Please always use all relevant information:

int MAXIMUM_TASK_DURATION_MINUTES = 3;

Can't remember how often I had some strange undocumented API and was wondering if it is minutes, seconds or millis.

Answer 3

THREE = 3 makes no sense

I can't imagine a good justification for having such a constant as THREE = 3 or THREE_MINUTES = 3 EDIT: there is a case, though it's language specific and pretty obscure; see the supercat's comments. However, in come contexts, and especially in tests, it may be useful to have a constant that represents some amount of... something, represented in a specific unit.

What can make sense

For example, if you need to test some computation on durations, or you need to sleep for a while, you might want to have something like

const int THREE_MINUTES_IN_MS = 3 * 60 * 1000;
...
sleep(THREE_MINUTES_IN_MS);

or

const int MINUTE_MS = 60 * 1000;
...
sleep(3 * MINUTE_MS);

or even

const int MINUTE_MS = 60 * 1000;
const int THREE_MINUTES_IN_MS = 3 * MINUTE_MS;
...
sleep(THREE_MINUTES_IN_MS);

What happens here is that the constant captures both the amount and the unit conversion, making it both easier to read and to contain the unit conversions in a place where it's easier to catch mistakes. That last point is important in my experience: if you see code sleep(3 * 60 * 100) you may guess that this is a mistake, but can't be sure, whereas MINUTE_MS = 60 * 100 is clearly a mistake. It's just more semantic information.

When it may be justified to use names like THREE_MINUTES_IN_MS

If the number is defined just because in a test you need to sleep for a while several times, in slightly different contexts, it may be preferable to use this sort of meaningless naming. It conveys a meaning actually: it clarifies that this is an arbitrary number that is not related to anything specific. You just decided that 3 seconds would be a good number to wait for "a little while". Or perhaps you are actually testing some computation on numbers, and want to make assertion 4 * millis(MINUTE_MS) == minutes(4) easier to read.

Another way it may help is that such a name is clearly disposable. If you realise that there is an unused constant FROBNICATOR_REQUEST_DELAY_SMALL it may make you uneasy - can you remove it? Should it be used somewhere? A constant THREE_MINUTES_IN_MS is clearly disposable, if it's unused, you delete it and never look back. And even more importantly, when reading, you never need to decipher its meaning, it should never surprise you.

Prefer saying what the number represents

Obviously, if the number has a meaning such as MAXIMUM_TASK_DURATION, you should use that name instead (though sometimes it may make sense to define it in terms of another constant - MAXIMUM_TASK_DURATION_MS = THREE_MINUTES_IN_MS.

Answer 4

This is indeed inadequate and should be flagged in review. To understand why, let's step back.

What's in a variable?

A variable (or constant) is an instance (value) of a given type. It conveys a variety of pieces of information through both its name and its type.

A variable is, first and foremost, a value. In your case, the underlying value is 3.

A variable with a number as an underlying value generally represents a quantity. A quantity is often the association of a number and a unit, though there are unitless quantities such as the number of iterations in a loop.

The unit can be conveyed through either type or name:

public static int THREE_MS = 3;
public static Milliseconds THREE = new Milliseconds(3);

Although the unit itself is not as important as the dimension:

public static Duration X = Duration::from_milliseconds(3);

It is notable that just because 2 variables have the same dimension does not mean that they interchangeable, and therefore it can be valuable to further refine a type-hierarchy -- when such a thing exists -- to represent specific kinds of values. This refinement can also be applied to unitless quantities, such as indices on a board: a row index is not a column index, and vice-versa.

Finally, a variable typically has a purpose which further restricts its applicability. You would not want to use the heartbeat_interval instead of the polling_interval, for example.

Applying to your example.

In general, I'd recommend employing a mix of type and name.

I myself prefer to err on the side of types, wherever practical, and routinely create "wrapper" types. In the above example, should I create a Board, I would have:

class RowIndex(int);
class ColumnIndex(int);

class Cell(...);
class Board(...);

function Board.get(RowIndex row, ColumnIndex column) -> Cell;

And this would help avoiding accidentally swapping one index for another.

I do use names, still, and thus going back to your previous example:

public static Duration REQUEST_INTERVAL = Duration::from_milliseconds(3);

Would be my favorite:

1. A dimension type precisely conveys the dimension of the quantity.
2. By using a dimension type, we can also precisely convey the unit of the number, at its point of construction.
3. Finally, the name conveys the purpose.

On the point of the name, note that I did not name it REQUEST_INTERVAL_MS nor REQUEST_INTERVAL_DURATION, there is not point in repeating in the name information provided by the type.

Applying to the original example.

The original (linked) example is:

int seconds = days * 24 * 60 * 60;

The error, here, is the Primitive Obsession. Using meaningful types allows having meaningful names.

As an example, C++ std::chrono::duration<...> can express both days and seconds resolution, it uses magic numbers in its formation, where the intent is clear, and never again:

using nanoseconds = duration<int64_t, std::ratio<1, 1'000'000'000>>;
using microseconds = duration<int64_t, std::ratio<1, 1'000'000>>;
using milliseconds = duration<int64_t, std::ratio<1, 1'000>>;
using seconds = duration<int64_t, std::ratio<1, 1>>;
using minutes = duration<int64_t, std::ratio<60, 1>>;
using hours = duration<int64_t, std::ratio<3600, 1>>;
using days = duration<int64_t, std::ratio<86400, 1>>;

And from there I can do:

auto const x = seconds{ days{ 3 } };

To have x be a duration expressed in seconds initialized by a number of days.