In various code reviews I perform on Java and Groovy code I often see magic numbers and other random Strings littered all over the code base.
A magic number, for example, is the 4.2 in below (Groovy) code snippet:
if (swashbuckle >= 4.2 ) { ... } |
What does 4.2 mean?
My advise is to extract such a number to a constant and give it a meaningful intention-revealingname, so that we can all keep understanding our code.
After refactoring into
if (swashbuckle >= MAX_ALLOWED_CAPACITY) { ... } |
even beginning developers understand the value of The Constant and start extracting values all over the place into constants. When I talk about enums it’s even more possible to write readable code, but quickly inexperienced developers quickly fall into the following traps.
Pitfall #1 Global Über-Constants File
A global constants file should be avoided as much as possible, such as
class Constants { private static final String PEACH_FLAME = "PFL" private static final int MAX_TOOGIT = 17 private static final int MIN_TOOGIT = 8 private static final String USER_NAME_AGE_PROPERTY = "age" private static final String USER_NAME_FLOPPY_PROPERTY = "floppy" private static final int CUSTOM_HYSLERIA_DONE = - 99 private static final List<String> WOBBA_RANGE = [ 'BZ, ' FLL ', ' BZZ'] // dozens of other constants... |
Quote on StackOverflow which summarizes it pretty well:
I would highly advise against having a single constants class. It may seem a good idea at the time, but when developers refuse to document constants and the class grows to encompass upwards of 500 constants which are all not related to each other at all (being related to entirely different aspects of the application), this generally turns into the constants file being completely unreadable. Instead:
- If you have access to Java 5+, use enums to define your specific constants for an application area. All parts of the application area should refer to enums, not constant values, for these constants. You may declare an enum similar to how you declare a class. Enums are perhaps the most (and, arguably, only) useful feature of Java 5+.
- If you have constants that are only valid to a particular class or one of its subclasses, declare them as either protected or public and place them on the top class in the hierarchy. This way, the subclasses can access these constant values (and if other classes access them via public, the constants aren’t only valid to a particular class…which means that the external classes using this constant may be too tightly coupled to the class containing the constant)
- If you have an interface with behavior defined, but returned values or argument values should be particular, it is perfectly acceptable to define constants on that interface so that other implementors will have access to them. However, avoid creating an interface just to hold constants: it can become just as bad as a class created just to hold constants.
A single class — such as our Constants example above — quickly becomes a bag of everything. The rookie developer thinks he’s following good (code review) advice by extracting magic numbers and magic Strings into constants, but the team quickly has a new maintenance burden on their hands.
If you find yourself (or your team) doing this, please put the constants in responsible owners e.g. user-related constants in the UserService
and wobba-related constants in theWobbaConverter
– whatever that is
Also read the part on enums in above comment, because constants aren’t the only kid in town. Sometimes my advice is to…
Prefer enums
If your constants can be well-modeled as an enumeration, consider the enum structure. Enums are more versatile than just plain constants; they are classes and can contain properties and methods.
Either inside a responsible parent class.
Prefer
class Person { enum Gender { M, F } String name Gender gender }
over
class Person { static final String GENDER_MALE = 'M' static final String GENDER_FEMALE = 'F' String name String gender }
Or as a separate class (if it becomes big) near the classes who use it. Good example of anenum
class with a functional name holding some associated (technical) data is e.g.
/** * Represents medicine domain codes. */ public enum MedicineCode { /** Diagnosis e.g. "Muscle damage". */ DIAGNOSIS("X357"), /** Units in medicinal context e.g. "cc/ml". */ MEDICINE_UNIT("X523"), /** * Cause codes for diagnosis = 'Masitis' e.g. "E.coli (ECO)". */ CAUSE_CODE("X536"), /** TreatmentType e.g. "Antibiotics". */ INTERVAL_TYPE("X520"), MedicineCode(String code) { this.code = code; } private final String code; public String code() { return code; } /** * Find a {@link MedicineCode} by given String code. * * @param code The code e.g. "X261" * @return found medicine code, or null */ public static MedicineCode findByCode(String code) { values().find { it.code() == code } } @Override public String toString() { return name() + "(" + code() + ")" } }
You should use enum types any time you need to represent a fixed set of constants. So, the rookie developer thinks he’s following good (code review) advice by extracting stuff into enums, encapsulating technical data, using a functional name, etc – but usually falls into
Pitfall #2 Defining Enums, Not Really Using Them Properly
So if you had originally the following method and invocation:
Medicine findMedicineForDomainCode(String code) // which you call like: String intervalTypeCode = "X520" findMedicineForDomainCode(intervalTypeCode)
and you would have introduced an enum like MedicineCode
(see above) encapsulating all these domain-specific, technical e.g. database”X…” codes (such as “X520”) then do NOT do this:
Medicine findMedicineForDomainCode(String domainCode) // which one keeps calling like: String intervalTypeCode = MedicineCode.findByCode("X520") findMedicineForDomainCode(intervalTypeCode)
I’ve seen teams doing it like this. Yes, there ís an enum type with values, but the team doesn’t quite understand what to do with them throughout the code.
First step is to refer to the enum directly. Often this is understood already initially by some rookie developers depending on whether or not they followed the Oracle Java Enum Types tutorial or something similar, but it usually results in something like this:
Medicine findMedicineForDomainCode(String code) // which one calls like: String intervalTypeCode = INTERVAL_TYPE.code() // WRONG! still using Strings here findMedicineForDomainCode(intervalTypeCode)
Having an enum means we now can type everything, including return types and method parameters with them.
Just using the enum as a container to hold a String isn’t why we’re doing it: to get better type safety and readability you should refactor all things in the code where you would use the domain code as a String to the MedicineCode
enum class.
Better:
// first refactor method parameter from String to MedicineCode Medicine findMedicineForDomainCode(MedicineCode code) // now just pass an enum value findMedicineForDomainCode(INTERVAL_TYPE)
Then, and only then, at the last possible moment, where you need the actual encapsulated String code (“X520”) – one can extract it from the enum value.
Hopefully this helps a bit in defining constants, and using enums. I couldn’t cover all possible other scenario’s of “valid” kinds of usage and OO advantages you could have with properly designed enum types, but hopefully this article will prevent someone new to Java from falling into described pitfalls.