Java Code Organization – Using Nested Public Classes for Constants

The source tree layout should reflect the architecture; as a corollary, a well-structured architecture can lead to a well-structured source tree layout. I suggest reading up on the POSA1 Layers pattern, attempting to fit your architecture into a layered structure, then naming each of the resulting layers, and using that as a basis for your source hierarchy. Taking a common three-tier architecture as a baseline:

• presentation/webService (present a web-service interface to our business logic)
• logic/* (business logic modules go in here)
• storage/sql (back-end storage APIs here - this uses a SQL interface to store to a database)
• util/* (utility code - usable by all other layers, but that does not refer outside util, goes here)

Note that the layers do not contain code directly, but rather are strictly used to organize modules.

Within a module, I use the following sort of layout:

• <module> (path to module directly; defines modular interface)
• <module>/impl/<implName> (a specific implementation of the modular interface)
• <module>/doc (Documentation for using the module)
• <module>/tb (unit-test code for the module)

where the <module> is located in the repository according to the layer to which it belongs.

There are two options that one has for dealing with specific, frequently occurring immutable objects. You can either instantiate them each time, or you can use a constant that is probably stuck in a final static in some class.

BigInteger for example has ZERO, ONE, and TEN.

However, this is an attempt to avoid instantation of new objects. Say you're adding the sum of some BigIntegers. You've got the choice of:

BigInteger sum = BigInteger.ZERO;
for(int i = 0; i < array.length; i++) {
  sum = sum.add(BigInteger.valueOf(array[i]));
}

vs

BigInteger sum = BigInteger.valueOf(0);
for(int i = 0; i < array.length; i++) {
  sum = sum.add(BigInteger.valueOf(array[i]));
}

vs

BigInteger sum = null;
for(int i = 0; i < array.length; i++) {
  if(sum == null) { sum = BigInteger.valueOf(array[i]); }
  else { sum = sum.add(BigInteger.valueOf(array[i])); }
}

The first code is simpler to understand and less error prone.

The second creates another object when its not needed - do this several times and you've got several BigInteger.valueOf(0) objects that need to get collected at some point.

The third is error prone - if the array is empty, sum comes out with a null.

However this should be remembered that this is trying to avoid instatiation of objects. Using it for things such as comparisons doesn't help.

if (myArray.length == Constants.ZERO_INT) 

This is some dangerous code. Sure, ZERO_INT is an Integer and it will get unboxed back to a 0. But if the myArray ever becomes something that returns an Integer? It may break depending on how ZERO_INT was defined (Integer ZERO_INT = 0 is less likely to, Integer ZERO_INT = new Integer(0) is more likely to). And things you get back from ORMs and the like are rarely boxed.

At a former employer, I had BigDecimal KM_IN_MI = BigDecimal.valueOf("1.60934"); defined because it was a value that kept getting used and removed code that would create it a few dozen times otherwise. That's a good constant - and its not one that would get auto boxed for you. Its also one that needs to stay consistent through multiple calculations.

Defining Integer.valueOf(0) fails at this test because it is one that would get auto boxed for you, is more likely to be buggy (the == works sometimes), and is already a constant 0.

The empty string is... kind of useless, other than its name. "" is easier to type and probably more convenient. That said, some tools will yell at you for having a string constant used multiple times as string constants - set up your static analysis to put "" into its exception list.

This is because Java does String interning. They take strings out of college and put them to work. At compile time, all the string literals are put into a specific pool of strings that are pre-defined. This can save memory and speed things up - and make things referentially equal ("foo" == "foo" is true, while "foo" == new String("foo") is false, but "foo" == (new String("foo")).intern() is true -- got all that?).

Defining some strings as constants means that you are not violating DRY. But for "", this is probably overkill, unnecessary and, frankly, a pain.

For final static primitives (byte, short, int, float, double, char, long, boolean), these are translated to literals at compile time (String also has this behavior though it is complicated with the interning pool). They do have uses, as can seen in the code for java.util.Calendar. However, extreme caution must be taken with this.

If your code looks like:

class A {
   final static int FOO = 1;
}

class B {
    private int field;
    public B { field = A.FOO; }
}

when it gets compiled, class B effectively becomes:

class B {
    private int field;
    public B { field = 1; }
}

There is no reference to A.FOO in the code. This is mentioned in Understanding Class Members:

If a primitive type or a string is defined as a constant and the value is known at compile time, the compiler replaces the constant name everywhere in the code with its value. This is called a compile-time constant. If the value of the constant in the outside world changes (for example, if it is legislated that pi actually should be 3.975), you will need to recompile any classes that use this constant to get the current value.

As these numbers get compiled into any class that uses them (rather than referencing the constant), it means if the value changes for some reason everything that uses it must be recompiled. Failing to do that means that you think you are using a given value from the new .jar but you are still using the old one.

There are a couple of ways to make sure you don't have this problem of mismatched constants.

The easiest (and hardest to enforce) is to say "once something is a public final static int answer = 42 it never changes." If you need a new answer, you make a new constant. If you need to change this value some day, too bad.

One is to make sure that the values don't get out of the package as a public final static and instead make it package level. This means that it isn't likely to be used in another .jar and changing it means recompiling the current .jar.

Another approach is to make the constant accessed via a getter. The getter doesn't get inlined at compile time, but can get inlined at JIT time. This means that you don't have to worry about if the constant in the other class changes because you are accessing it via a getter at runtime rather than compile time inlining.

As this is a bit complex and has been the source of at least one bug that I've fought (and oh, what an epic fight it was - it works on my machine, doesn't work in the dev environment, works on test, but not on production (but differently than how it didn't work in dev); because different versions of a .jar were present on each one and the final static int was different) -- one should be wary of these values if they have at all a chance to change with future versions. There is also some more esoteric breaking of case statements where two of them could have the same value. There are some idioms to avoid this mentioned at the end of section 13.4.9 of the JLS though it has the caveat of "We note, but do not recommend..."

I should point out that final static {primitive} is not a pool at all in the language of patterns. It is a way of invoking a compiler optimization for DRY at the cost of closer coupling between the classes and the possibility of errors if that coupling doesn't mean all the classes are recompiled at the same time when the 'constant' changes.

Constant pools of objects are there for performance issues. Look at the IntegerCache in the Integer class (its a Flyweight Pattern). Or the proper use of String interning.

But that speed performance gain is unlikely of any use if the code that is written is harder to understand and more likely to have a bug introduced in it.

When dealing with non-mathematical constants and primitive types, make sure that these values are constant to avoid some rather hard to track down bugs (see this explanation of one such 'bug').

Write what is understandable. Unless demonstrated otherwise, programmer time is more valuable than cpu time.