Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Magic numbers are bad. I totally agree. But there's a magic number I find hard to fix:

'100' is a magic number.

Consider this code:

public double getPercent(double rate) {
    return rate * 100;
}

public double getRate(double percent) {
    return percent / 100;
}

Sonar will raise 2 violations, one for each use of 100. I could replace 100 with a constant variable, but what would be a good name for it? Is that really a good idea? If somebody ever changes its value that will be a disaster. I could also just add // SONAR to all the lines that use 100.

What is the best practice for dealing with the magic number 100?

UPDATE

From the answers, the most useful bit for me was this:

Practical Magic Number rule: A literal is a not a magic number if the most meaningful variable name for it is the same as the spoken name of the literal.

So by this logic, 100 is NOT a magic number.

However, to make the violation go away, I decided to replace the 100 with a constant:

public static final double HUNDRED = 100;  // change it and I'll kill you

In my real project there are many lines using 100, and if I put // NOSONAR on all those lines that might cover up other potential problems that other developers might inadvertently add later.

Not sure if there will be any real benefits using this constant. A small one may be that when I do git grep 100 I see a lot of matches from resource files in the project, while git grep HUNDRED turns up just the the Java code that uses this.

share|improve this question
9  
I've written a rant on stupid linters over on programmers. Adding a named constant for the 100 here would just be obfuscation. However, I feel you aren't looking for a code review of that very small snippet, but for best practices regarding such cases, which unfortunately is an opinion-based question. –  amon Mar 11 at 9:24
5  
What is adding insult to injury is the underlying reason: you shouldn't have magic numbers but meaningful identifiers. But that's exactly why there is a meaningful identifier on the previous line: getPercent ! –  MSalters Mar 11 at 15:00
5  
Well, it will allow you to more easily change it if the percentage of 1 ever changes... ;) –  Jon Hanna Mar 11 at 16:38
9  
You could name it PERCENT_TO_RATE_RATIO = 100 –  Cruncher Mar 11 at 18:05
14  
HUNDRED is absolutely as useless as 100, and potentially even more harmful since someone could ignore your comment and change it. If anything it proves that it matches the "practical magic number rule" and shouldn't be a variable, final or not. –  Kevin Mar 11 at 20:35

8 Answers 8

up vote 45 down vote accepted

Speaking as a human programmer (i.e. I am not Lint software), your use of "100" there looks fine to me.

Wikipedia has an article (without citations) titled Accepted limited use of magic numbers: IMO your "100" is in the same category as these other "accepted" magic numbers.


This Wiki describing magic numbers says two things.

Firstly,

Practical Magic Number rule: A literal is a not a magic number if the most meaningful variable name for it is the same as the spoken name of the literal.

That's applicable here: you're looking for a named constant like HUNDRED or CENTUM.

Secondly, it also suggests loading "magic" numbers (e.g. a "discount rate") from a configuration file:

static final double DISCOUNT_PERCENT = getProperty( "sales.discount_percent" );
static final double DISCOUNT_FACTOR = 1 - (DISCOUNT_PERCENT / 100);

// ...

salePrice = DISCOUNT_FACTOR * regularPrice;

Note that though this example code carefully loaded DISCOUNT_PERCENT from a configuration, the "100" used to calculate the DISCOUNT_FACTOR is hard-coded.


If you use "100" instead of HUNDRED, it's easier for a programmer to understand, and to verify that it's correct.

IMO the only benefit to using HUNDRED is to find the several methods which use the same magic number (in your example it's used by getPercent and getRate).

share|improve this answer
8  
@janos then you can do return rate * 2 * 2 * 5 * 5;. Much easier since it doesn't contain magic numbers. Much easier to read as well since it's been factorized /s –  WernerCD Mar 11 at 17:31
4  
I don’t even agree with the “only benefit” you mention: searching for “HUNDRED” is no easier than searching for “100”. 100 is not a magic number here and there’s no reason to reduce clarity in order to kowtow to some linter. –  bdesham Mar 11 at 21:34
6  
@WernerCD: Sorry, only 0,1 and 2 are not magic. ITYM (rate << 2) * (2*2+1) * (2+2+1). –  MSalters Mar 12 at 8:30
11  
@WernerCD Too complicated! 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1‌​+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+‌​1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1 ftw. –  David Richerby Mar 12 at 11:45
4  
My first thought was that surely you shouldn't be converting between rate and percentage in code since you'd use rate for all calculations and percentage only when displaying to the user. Does Java not have any nice number formatting methods that will format a double as a percentage string? –  Chris Mar 12 at 16:29

Although 100 should be fine in source-code, I'm surprised nobody has offered the most readable alternative yet. This should be acceptable for both humans and lint-code:

Define your constant PERCENT=0.01.

Then, when you need to do a conversion:

rate = discount*PERCENT

or

discount = rate/PERCENT

This can completely eliminate your short functions (which are, indeed, trivial). You could have additional constants PERMILLE, PPM, PPB, etc., and it should be obvious for humans what is happening.

share|improve this answer
    
+100 Or if you really must make it clear what the constant means, name it PER_CENT as in "per one hundred"--the definition of percent. –  David Harkness Mar 12 at 18:37
5  
Created an account just to vote this up. This is way better than the other answers. –  bjb568 Mar 13 at 2:24
1  
very good, but there are some caveats, I believe, with the internal representation... you may need to have one per types of data (float, double float, etc)... 100 (or a constant valued at 100) is safer to use.See: Java Double value = 0.01 changes to 0.009999999999999787 or why-not-use-double-or-float-to-represent-currency or rounding-errors –  Olivier Dulac Apr 3 at 16:12
1  
I can't really call this more readable. In particular, discount = rate/PERCENT feels rather contorted. Without very clear names, and perhaps even with, it'd be way too easy to forget whether PERCENT is 0.01 or 100. Perhaps if it were called ONE_PERCENT or something, i dunno...but as is, it's more magical than if it were left a "magic number". –  cHao Jun 26 at 1:33

Some numbers are called 'magic' because it is unclear where they come from. I think in this particular case, it is clear that 100 originates from the definition of percent. However, if you wish you can define a constant PERCENTS_IN_UNIT_RATE=100 instead of using it directly.

Violations from code analysis tools are really only suggestions and it is okay to disagree with them. If in doubt ask other programmers who work on the same project, or toss a coin and move to the next task! :-)

share|improve this answer
4  
I don't think HUNDRED is any more readable than 100. Yes, it will satisfy the immediate tool complaints, but you probably use the tool to raise code quality, and as I mentioned it does not do so in my opinion. MAX_PERCENT is better but still may raise questions. PERCENT_MAX sounds similar to INT_MAX and may be slightly better yet. PERCENTS_IN_UNIT_RATE is imho most understandable but is also too long. Hence, I'd vote for keeping literal 100. –  mkalkov Mar 11 at 10:39
1  
MAX_PERCENT seems misleading in that it implies that the 100 is just an arbitrary constant that could be changed any time. It would seem to me that outside of dodgy accounting practices, 100 is pretty immutable :-) –  microtherion Mar 11 at 10:51
1  
A100PERCENTS or ONE100PERCENTS or PERCENTS_100 or X100PERCENTS would all work IMO. I like these because they include numeric 100, and also include word percent, making both numeric value and context apparent. –  hyde Mar 11 at 11:13
1  
If I buy a goat for $1 and sell it for $8, then I've sold it for 800% of its original price. MAX_PERCENT should probably be a constant equal to DBL_MAX. Just use 100 and avoid confusion. –  Brendan Mar 11 at 17:39
    
I have no idea where RATE in PERCENTS_PER_UNIT_RATE came from. Because you're working on an interest rate? But that has absolutely no effect on conversion between percentage and scale. I'd suggest PERCENT_PER_UNITY or something like that. –  Ben Voigt Mar 11 at 22:12

I am on the fence about the 100 being a named constant or not.

I habitually make magic numbers named constants, but not all the time.... it is not unusual for me to have constants called HUNDRED or MILLION. I also regularly have trivial numbers as magic-number constants ....

But, in this case, and what I particularly want to draw attention to, is that your value should be a floating-point, not an integral value.

When you have constants they should be in the most convenient form for users to understand. For example, your value 100 should really be 100.0 which makes the fact that it is a double value obvious.

share|improve this answer
6  
When you use a constant called HUNDRED, is it 100 or 100.0? –  Joonas Pulakka Mar 11 at 19:09
    
@JoonasPulakka I would make it private static final double HUNDRED = 100.0; –  rolfl Mar 11 at 19:10
3  
Ok, but that's just one possibility. HUNDRED could as well be a BigInteger, or BigDecimal, or byte, just to mention a few. With number literals, the type is immediately obvious. –  Joonas Pulakka Mar 12 at 7:36
4  
If you end up dealing with large numbers a lot, i think things like MILLION could come in handy just because it could be very easy to mistake 1000000 for 10000000 etc –  DLeh Mar 12 at 20:09
1  
If you use HUNDRED in lots of places, then, when you need to change it in a subset of these places and try to change the constant... you're messed up. This would be no better than doing something like sed 's/100/101/g' for all of your project codebase. –  Ruslan Mar 14 at 6:27

Others have already stated why they believe that 100 is an acceptable constant here. This answer shows how you can get Sonar to accept that.

Under your-sonar-domain/profiles/, you can edit quality profiles, and the rules in them. The "Magic Number" rule is customisable, you can specify certain numbers for the rule to ignore. Simply specify "100" as a number to ignore.

Change ignoreNumbers to include "100!

share|improve this answer

It technically is since both those 100's have the same context. Similar to me using 32 as a width in several different places. It depends how nitpicky you want to be. In this case I might use something like MAX_PERCENT. It does help with readability as well since when I see that I know the calculations have to do with percentages.

share|improve this answer
1  
I would be rather amused if I saw code that used MAX_PERCENT –  Navin Mar 12 at 4:38
    
Poor attempt at brevity I guess. Though being amused by something helps remember it. –  0xFADE Mar 12 at 18:12

If forced to use a symbol, I might call it N100. That way you can tell what its value is. A long time ago I saw this done for common constants. And negative numbers were prefixed with "M". Why did they do this? Because the computer instruction set couldn't load numbers in line ("immediate"); they had to be fetched from a memory location. And the assembler used symbols to refer to those locations.

share|improve this answer

I think 100 IS a magic number here.

Percentage is vastly used, but also per mille (1/1000) is often used, and basis point (1/10.000) and percent mille (1/100.000) are used enough to have their own name.

I would change your function to getPerPart(value, part) and getFromPart(value, part), maybe with a default getPerPart(value) and getFromPart(value), which use DEFAULT_PART_VALUE = 100.

Also, you may still want to write a getPercent, in which case the use of hardcoded 100 is fine.

share|improve this answer
    
You are really going to make functions for multiplication and division? Why not allow your formatter to display numbers how the user wants to see them? You're not going to use these functions in calculations. –  Navin Mar 15 at 5:55

protected by Malachi 10 hours ago

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.