Numbers do not come with context information to describe what they refer to. 24 can be the number of hours in a day or the number of months in two years depending on the context. By replacing the magic number with a human-readable string, the real intent becomes obvious.
For example, nobody noticed that the '8' in the below code represented weeks and no longer matched the variable name:
long threeWeeksAgo = exportDate - ((1000L*60*60*24*7)*8);
A simple replacement of some of the values made the code readable:
private static final long MSECS_PER_WEEK = 1000L*60*60*24*7;
private static final long ROLLBACK_WEEKS = 8L;
:
long rollbackStartTimeMsecs = exportFileDateMsecs
- (MSECS_PER_WEEK * ROLLBACK_WEEKS);
Note that the MSECS_PER_WEEK variable still has some magic numbers in it, but the number of milliseconds per week is unlikely to ever change. Since it isn't going to change, it could acceptably be replaced with 604800000L – either is fine. Also in the latter readable code, exportDate was changed to exportFileDateMsecs and threeWeeksAgo was changed to rollbackStartTimeMsecs.
Exception: Where magic numbers represent a real number rather than a concept they should not be replaced with a static variable.
This exception is best illustrated with with an example. e.g. the initial size of a StringBuilder is clear and obvious in the following constructor call and requires no change:
StringBuilder buf = new StringBuilder(1024);
But in this case the developer felt obliged to replace it and created a definition which gave no flexibility to change the buffer size:
public static final int SIZE_1024 = 1024;
:
StringBuilder buf = new StringBuilder(SIZE_1024);
Worse, it requires three modifications to change a simple concept like the buffer size. It would be much better to describe 1024 for what it will be used for rather than for what it is:
public static final int BUFFER_SIZE = 1024;
:
StringBuilder buf = new StringBuilder(BUFFER_SIZE);
But this is still a round-about way to define a buffer size, because the developer needs to check the top of the file to see what the value is. The best approach was the original, simplest one:
StringBuilder buf = new StringBuilder(1024);
