What is a magic number, and why is it bad? [closed]

Question

What is a magic number?

Why should it be avoided?

Are there cases where it's appropriate?

Answer 1

A magic number is a direct usage of a number in the code.

For example, if you have (in Java):

public class Foo {     public void setPassword(String password) {          // don't do this          if (password.length() > 7) {               throw new InvalidArgumentException("password");          }     } } 

This should be refactored to:

public class Foo {     public static final int MAX_PASSWORD_SIZE = 7;      public void setPassword(String password) {          if (password.length() > MAX_PASSWORD_SIZE) {               throw new InvalidArgumentException("password");          }     } } 

It improves readability of the code and it's easier to maintain. Imagine the case where I set the size of the password field in the GUI. If I use a magic number, whenever the max size changes, I have to change in two code locations. If I forget one, this will lead to inconsistencies.

The JDK is full of examples like in Integer, Character and Math classes.

PS: Static analysis tools like FindBugs and PMD detects the use of magic numbers in your code and suggests the refactoring.

Answer 2

A Magic Number is a hard-coded value that may change at a later stage, but that can be therefore hard to update.

For example, let's say you have a Page that displays the last 50 Orders in a "Your Orders" Overview Page. 50 is the Magic Number here, because it's not set through standard or convention, it's a number that you made up for reasons outlined in the spec.

Now, what you do is you have the 50 in different places - your SQL script (SELECT TOP 50 * FROM orders), your Website (Your Last 50 Orders), your order login (for (i = 0; i < 50; i++)) and possibly many other places.

Now, what happens when someone decides to change 50 to 25? or 75? or 153? You now have to replace the 50 in all the places, and you are very likely to miss it. Find/Replace may not work, because 50 may be used for other things, and blindly replacing 50 with 25 can have some other bad side effects (i.e. your Session.Timeout = 50 call, which is also set to 25 and users start reporting too frequent timeouts).

Also, the code can be hard to understand, i.e. "if a < 50 then bla" - if you encounter that in the middle of a complicated function, other developers who are not familiar with the code may ask themselves "WTF is 50???"

That's why it's best to have such ambiguous and arbitrary numbers in exactly 1 place - "const int NumOrdersToDisplay = 50", because that makes the code more readable ("if a < NumOrdersToDisplay", it also means you only need to change it in 1 well defined place.

Places where Magic Numbers are appropriate is everything that is defined through a standard, i.e. SmtpClient.DefaultPort = 25 or TCPPacketSize = whatever (not sure if that is standardized). Also, everything only defined within 1 function might be acceptable, but that depends on Context.