3

Originally, in most languages, I would like to put constants into a class like that:

public class AppConstants{
    public static final double HEIGHT_MAX=20.0;
    public static final int COUNT_MAX=100;
    public static final double WEIGHT_MAX=30.0;
} 

but I found it is violating single responsibility principle, for example, some classes use HEIGHT_MAX only, while the others may use both HEIGHT_MAX and COUNT_MAX but never use WEIGHT_MAX.

So my question is, is it a good practice to define each class for each variable :

public class HEIGHT_MAX{
    public static final double value=20.0;
}

public class COUNT_MAX{
    public static final double value=100;
}

public class WEIGHT_MAX{
    public static final double value=30.0;
}

so that a higher level class imports that class only when it requires that constant?

  • Those classes haven't behaviour. They are just containers of data. Which should be separated based on their relation. Sometimes it is ok to have two constants with same value but different name/class container. – Fabio Apr 20 '17 at 3:44
  • Aside from probably being a little vague with the member names, I would not bother refactoring your AppConstants class at all. SOLID only applies to behavioral classes, not data structures. – Graham Apr 20 '17 at 12:35
  • 4
    This is a perfect example of the danger of cargo cult design; following a well intended "principle" to its logical end results in absurd designs. – Frank Hileman Apr 23 '17 at 22:46
12

Well, first, classes' names are usually singular by convention.

Then, constants should exist in their pertinent classes.

For example:

public class Building{
    public static final integer MAX_HEIGHT=300;
    ... more constants and behavior ...
}

public class WindowPane{
    public static final integer MAX_HEIGHT=400;
    ... more constants and behavior ...
}

public class PlantStalk{
    public static final integer MAX_HEIGHT=100;
    ... more constants and behavior ...
}

Building.MAX_HEIGHT is more intention-evident than Constants.MAX_HEIGHT.

Also, evidently, Building.MAX_HEIGHT, WindowPane.MAX_HEIGHT and PlantStalk.MAX_HEIGHT have nothing to do which each other, so it is not like they are "violating SRP".

Having the constants in their pertinent classes also solves both the "should I have a constants-only class" dilemma and the "should I have a class with just one constant and no behavior" dilemma.

Related answers:

Using nested public classes to organize constants

Is it a bad practice to have an interface to define constants?

  • 2
    +1. It's usually always possible to find some class which a constant is a property of, so it should be a public static constant of that class. – Philipp Apr 20 '17 at 13:29
7

Let's think about this in terms of the abstraction(s) available to the client programmers.

AppConstants.HEIGHT_MAX isn't the greatest interface.

First, (assuming that height, width, and count are indeed related) we need to at least name the class something relevant to the domain, something that indicates that all the constants related.

Second, you need to specify some units somehow. Height in inches vs. height in cm, for example. (It's too bad that many common programming languages don't offer specific (type system) features for that.)

Ideally of course, the now-well-named concept (instead of AppConstants) is also placed in a namespace that helps convey its meaning.

To get to the answer for your question, all in all, IMHO, it is better to have a good, unified, and useful abstraction than to separate these items for the sake of (import overhead masquerading as) single responsibility. The unified abstraction collecting the constants is a single responsibilty, whereas the individual constants alone make impoverished abstractions.

Regarding the alternatives, I would say that the overhead of managing multiple classes/imports for those clients who need more than one of the constants, I think, outweighs the (virtually non-existent) overhead of importing more constants than necessary in one domain-oriented class for those clients that need only one of the constants.

You'll probably also find that some behaviors are desirable within the abstraction. Conversions, etc... I would still argue that this makes a better and more complete abstraction and splitting things up hurts more than helps.

1

Is something like HEIGHT_MAX really an application level constant? Surely the maximum height is related to some specific data or functionality, and should be expressed closer to that? Like in a class-level constant. Or perhaps it ought to be located in configuration and then passed in when initializing the relevant object.

It's hard to say anything definitive without knowing the use case, but I have a strong suspicion that underneath your conundrum lies a bigger design issue.

0

+1 on renaming. Consider, what do the constants relate to conceptually/semantically. That should be the grouping principle, not the mechanics of importing etc.

So, HEIGHT_MAX of what? WEIGHT_MAX of what? You get my drift. The "Important_Thing" these relate to is the most useful information.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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