Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
1  
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. –  KChaloux Oct 26 '12 at 21:34

6 Answers 6

up vote 9 down vote accepted

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 hundres of costants, either grouped into inner clases 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 methods signatures relative to result sets, and implemented 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 hundres of costants, 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.

share|improve this answer
    
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. –  curioustechizen Aug 29 at 11:59
    
+1, but UI constants probably should not be in business models like BankAccount. They belong in some UI class. –  kevin cline Aug 29 at 15:35

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.

share|improve this answer
    
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. –  FrustratedWithFormsDesigner Oct 26 '12 at 20:53
4  
A constants class that's constantly changing... there's something zen in there, if you dig hard enough. –  KChaloux Oct 26 '12 at 21:32
2  
@FrustratedWithFormsDesigner: UI-field IDs? Don't know where to find a value? Sounds like a huge pile of WTF. You have my sympathy. –  kevin cline Oct 27 '12 at 15:55

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.

share|improve this answer
    
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 :) –  Goran Jovic Oct 26 '12 at 20:28
1  
@GoranJovic Well every anti-pattern is useful in some way (or at least convenient), it wouldn't become a pattern if it wasn't. –  Yannis Rizos Oct 26 '12 at 20:40

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.

share|improve this answer
    
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? –  FrustratedWithFormsDesigner Oct 26 '12 at 21:36
    
@FrustratedWithFormsDesigner: It depends on how much effort the incremental compilation mechanism spends on modeling dependencies. –  Michael Borgwardt Oct 26 '12 at 21:43

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.

share|improve this answer

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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