Code Smell of the Week, Design Patterns

Code Smell of the Week : Magic Numbers

if(order.getStatus() != 1)

First look at this code tells you that an order with a particular status is not displayed. But what is that particular status ? When is it changed ? Who the hell wrote this ? These are the frustrating questions that pop-up in the programmers head. What follows is a quest to find the creator of the code by navigating the Maze (a.k.a. The Code Repository). The creator may also have ‘moved on’ which leads to further pain.

The problem originates from the code not being self-explanatory as the meaning of  “1” is unclear. It’s meaning can magically change the entire meaning of the code. Thus “1” becomes a magic number.

The solution is simple yet elegant.

final int CANCELLED = 1;
if(order.getStatus() != CANCELLED)

This makes it crystal clear that an order which is cancelled will not be added to the list and hence not displayed. The value “1” was extracted into a constant which has a meaningful name. This small change improves readability and makes it easy for modification by  other people. Further the value “1” can be used with a different meaning in  the same code without creating more confusion by simply defining another constant. For example if only orders with cost greater than 1 Dollar can be shipped, then

final int CANCELLED = 1;
if(order.getStatus() != CANCELLED

The exact implementation of the constant is specific to the language. In JAVA it is usually a class variable

private static final int CANCELLED = 1

or it is placed in a utility class to be accessed across the application.

Thats all folks ! Keep sniffing for smells and see you next week.

Code Smell of the Week is a blog series where I attempt to de-mystify a code smell every week and have some fun along the way.

All examples are coded in JAVA owing to its popularity.


2 thoughts on “Code Smell of the Week : Magic Numbers

    • Hi Scream,
      Thanks for your comment. Yes Enums are one way of representing the constants. They are generally used when there are several related constants. e.g life-cycle of an item (ORDERED,SHIPPED,DELIVERED etc)
      However for something such as MINIMUM_AMOUNT_REQUIRED_TO_SHIP_ORDER in the example a simple constant is sufficient as it has no other related constants.
      In the end both are different solutions of the same problem.

What do you think ?

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s