Using nested public classes to organize constants

Question

I'm working on an application with many constants. At the last code review it came up that the constants are too scattered and should all be organized into a single "master" constants file. The disagreement is about how to organize them. The majority feel that using the constant name should be good enough, but this will lead to code that looks like this:

public static final String CREDITCARD_ACTION_SUBMITDATA = "6767";
public static final String CREDITCARD_UIFIELDID_CARDHOLDER_NAME = "3959854";
public static final String CREDITCARD_UIFIELDID_EXPIRY_MONTH = "3524";
public static final String CREDITCARD_UIFIELDID_ACCOUNT_ID = "3524";
...
public static final String BANKPAYMENT_UIFIELDID_ACCOUNT_ID = "9987";

I find this type of naming convention to be cumbersome. I thought it might be easier to use public nested class, and have something like this:

public class IntegrationSystemConstants
{
 public class CreditCard
 {
 public static final String UI_EXPIRY_MONTH = "3524";
 public static final String UI_ACCOUNT_ID = "3524";
 ...
 }
 public class BankAccount
 {
 public static final String UI_ACCOUNT_ID = "9987";
 ...
 }
}

This idea wasn't well received because it was "too complicated" (I didn't get much detail as to why this might be too complicated). I think this creates a better division between groups of related constants and the auto-complete makes it easier to find these as well. I've never seen this done though, so I'm wondering if this is an accepted practice or if there's better reasons that it shouldn't be done.

• KChaloux: I rather like the idea. I found myself doing something similar in C++ when I wanted C#-style enum-like scoping and behavior, but with meaningful values (I think the idea arose because I'd gotten used to using case classes in place of enums in Scala). Granted, this was a personal project for fun, and not a team effort.

Answer 1

I don't agree with either of the two proposals.

Constants should be in their pertinent classes, not in an all-constant class in either of the two forms proposed.

There shouldn't be constants-only classes/interfaces.

A class CreditCard (not an internal class) should exist. This class/interface has methods relative to credits cards as well as the constants UI_EXPIRY_MONTH and UI_ACCOUNT_ID.

There should exist a class/interface BankAccount (not an internal class). This class/interface has methods relative to bank accounts as well as constant UI_ACCOUNT_ID.

For example in the Java API every class/interface has its constants. They are not in a Constants class/interface with hundreds of constants, either grouped into inner classes or not.

For example, these constants pertinent to result sets are in the interface ResultSet:

static int CLOSE_CURSORS_AT_COMMIT 
static int CONCUR_READ_ONLY 
static int CONCUR_UPDATABLE 
static int FETCH_FORWARD 
static int FETCH_REVERSE 
static int FETCH_UNKNOWN 
static int HOLD_CURSORS_OVER_COMMIT 
static int TYPE_FORWARD_ONLY 
static int TYPE_SCROLL_INSENSITIVE 
static int TYPE_SCROLL_SENSITIVE 

This interface has method signatures relative to result sets, and implementing classes implement that behavior. They are not mere constant-holders.

These constants pertinent to UI actions are in the interface javax.swing.Action:

static String ACCELERATOR_KEY 
static String ACTION_COMMAND_KEY 
static String DEFAULT 
static String LONG_DESCRIPTION 
static String MNEMONIC_KEY 
static String NAME 
static String SHORT_DESCRIPTION 
static String SMALL_ICON 

Implementing classes have behavior relative to UI actions, they are not mere constant holders.

I know at least one "constants" interface (SwingConstants) with no methods but it doesn't have hundreds of constants, just a few, and they are all related to directions and positions of UI elements:

static int BOTTOM 
static int CENTER 
static int EAST 
static int HORIZONTAL 
static int LEADING 
static int LEFT 
static int NEXT 
static int NORTH 
static int NORTH_EAST 
static int NORTH_WEST 
static int PREVIOUS 
static int RIGHT 
static int SOUTH 
static int SOUTH_EAST 
static int SOUTH_WEST 
static int TOP 
static int TRAILING 
static int VERTICAL 
static int WEST 

I think constants only classes are not good OO design.

• curioustechizen: While I agree that constants only classes are in most cases a sign of bad design, there are legitimate uses. For instance, the constant doesn't always "belong" to a particular class. Keys for Intent extras in Android programming are one such concrete example.
• kevin cline: +1, but UI constants probably should not be in business models like BankAccount. They belong in some UI class.

Answer 2

it came up that the constants are too scattered and should all be organized into a single "master" constants file.

This is the 100% wrong solution to the problem of constants being "hard to find". It's a fundamental error (unfortunately all too commonly perpetrated by programmers) to group things by technical criteria like being constants, enums or interfaces.

Except for layer architectures, things should be grouped by their domain, and constants all the more so since they are very low-level elements. Constants should be part of the classes or interfaces that use them as part of their API. If you can't find a natural place for them to live, you need to clean up your API design.

Additionally, a single huge constants file kills development speed in Java because constants are inlined into the classes that use them at compile time, so any change forces a recompilation of all those files and all that depend on them. If you have one file with constants that are used everywhere, you largely lose the benefit of incremental compilation: watch Eclipse lock up for 15 minutes as it recompiles your entire project for every change to that file. And since that file contains all constants used anywhere, you'll be changing it quite often. Yes, I am speaking from personal experience.

• FrustratedWithFormsDesigner: I wasn't aware of the potential impact on the development environment, and I guess the nested class approach won't help much with that, right?
• Michael Borgwardt: @FrustratedWithFormsDesigner: It depends on how much effort the incremental compilation mechanism spends on modeling dependencies.

Answer 3

You are right. Your suggestion allows a programmer to import static Constants.BankAccount.* and eliminate countless repetitions of 'BANKACCOUNT_'. Concatenation is a poor substitute for actual scoping. That said, I don't hold out a lot of hope for you to change the team culture. They have a notion of proper practices, and aren't likely to change even if you show them 100 experts that say they are wrong.

I'm also wondering why you have a single 'Constants' file at all. This is very bad practice, unless these constants are all related somehow and are very stable. More typically it becomes a kind of kitchen-sink class that changes constantly and is depended on by everything.

• FrustratedWithFormsDesigner: We currently have one Constants file that contains most of the UI-field ID constants, and a couple of other constants files that are subproject-specific. But now, the subproject-specific constants are needing to be used by other related subprojects so that's what prompted the idea to merge them all to one "master" constants file. Which is better than now because sometimes I don't even know which constants file to refernece for a value, and someone else created a duplicate of a constant because they couldn't find it in a different subproject's constants file.
• KChaloux: A constants class that's constantly changing... there's something zen in there, if you dig hard enough.
• kevin cline: @FrustratedWithFormsDesigner: UI-field IDs? Don't know where to find a value? Sounds like a huge pile of WTF. You have my sympathy.

Answer 4

Both approaches work, and that's what's important at the end of the day.

That said, your approach is common enough to be considered a pattern, or more accurately a good enough improvement over the constant interface anti-pattern. I don't see what's complicated about it, but that's a discussion you should have with your team.

• Goran Jovic: For a long enough list of constants, I'd even prefer constant interfaces over having them all in the same class. It's dangerously possible to choose the wrong one from auto complete. Of course, that depends on how long is long enough :)
• yannis: @GoranJovic Well every anti-pattern is useful in some way (or at least convenient), it wouldn't become a pattern if it wasn't.

Answer 5

One of the challenges with the long list of constants is finding the "correct" constant to be using. Invariably, someone tacks a constant in at the end of the line instead of adding it to the group it belonged to.

Which brings up another point to discuss with your team - there already is a de facto categorization of the constants through their names. So it really shouldn't be a big shift for them from current state to what you're proposing. Using auto-complete makes constant look-ups even easier.

If anything, your suggestion helps discourage using the "wrong" constant even though it might fit in a particular situation. By wrapping the constants within a representative class, it will encourage the developers to think through if they should be using it. For example, if I pull CreditCard.SomeConstant for general usage, I really should wonder why there isn't a General.SomeConstant for that situation instead.

I've been on projects with monster amounts of constants, and I would have preferred to have the method you're suggesting. It's cleaner, more direct, and it helps avoid inadvertent usage.

Answer 6

I do this occasionally (in C#), particularly if I need to program against / parse a well-known and fixed XML structure or something similarly nested and string heavy...for instance a custom config block parser.

I build out a nested class structure matching the hierarchical structure of the XML w/ the class names corresponding to the elements containing a const string "ElementName" property and a similar property corresponding to each Attribute (if any).

It makes it very easy to program against the corresponding Xml.