6

I've been tasked with researching and making suggestions about refactoring/cleaning up some existing code in our code base. Presently, many of our projects depend on a Constants class, which contains all of the constants used in the library. While on one hand I get this choice - this object has a clear, defined purpose that's unique - I feel like it couples the code together too tightly.

For example, I have a library ImportXmlClasses. Within that library, there is the Constants class. In many cases, things that depend on ImportXmlClasses wind up knowing about this Constants class, and other Library's Constants class as well.

Should I suggest this class be removed?

  • 4
    I think you need to pick your battles. Figure out what the most important things are, the things that will give you the biggest bang for the buck, and focus on those. See also Parkinson's Law of Triviality. – Robert Harvey Dec 23 '16 at 19:27
  • Are these constants actually configuration parameters? – Richard Tingle Dec 23 '16 at 21:02
  • @RichardTingle They vary. Some are, others aren't. – Adam Wells Dec 26 '16 at 20:32
  • @Adam an argument could be made for having all configuration parameters in one place (although this is usually a separate configuration file that can be changed without recompiling). I agree that having the others there is extremely odd – Richard Tingle Dec 27 '16 at 9:35
8

The problem isn't that you shouldn't have a constants class. It's having a class that you think of as "our constants class". If I was a young new developer on you team and I saw a "constants class" I might think "oh, so this is where to put that PI constant I need. I'll put it right next to this "path" constant since they both start with p.

Whats wrong with that? Imagine what this class looks like in 5 years.

Give this thing a single responsibility and a name that reflects that. You'll find less clutter in it later.

If that can't be done because there is to much mixed clutter in there already then start kicking things out.

Constants that exist to support the same thing should stay together. Constants that support different things should be separated.

That should allow those that change together to stay together and those that change independently to live separately.

Otherwise you've just put a junk drawer in the heart of your application. Not exactly an elegant design.

  • I'd also add that please please have the constants in the class accessed in a static way referencing the class name plus the constant name. All too often (in Java, say) I've seen a class import multiple "constants" classes and reference the values by name, so that it's not immediately obvious to see in which class the value lives. It's not usually difficult to find out, but you take a small pain that you have to deal with daily and it quickly becomes a big pain. A little extra time taken early can really help – Andrew Dec 23 '16 at 23:14
0

If you could give a bit more context around the domain and the nature of the constants, we would probably be able to help a bit more.

But, in most cases, a constant is part of a specific domain. As a result, this constant should reside in a specific class (which encapsulates the whole logic of this domain). Ideally, you should have no Constant classes. Instead, you should structure your system as a set of classes, where the required constants are being distributed to these classes and each class executes specific logic based on these constants, exposing this logic via a well-defined API to users of this class (but again ideally, without exposing the constants themselves).

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.