An anti-pattern I have observed in some .NET projects, though it could equally be relevant in any object-oriented environment, is placing all constants for a project (or even a solution) in a single class file (e.g. Constants.cs
).
There are two major problems with this:
1. Cohesion
The constants very often have nothing in common with each other (except that they are constants). Some developers can be tempted to place things together only because they are the same type of thing (constant, enum, interface etc.). This kind of thinking should be avoided as it leads to poor cohesion.
Kent Beck said it best…
“Things that change at the same rate belong together. Things that change at different rates belong apart.” — Kent Beck
2. Encapsulation
When all constants in a solution are placed in a single class file the constants must be defined as public to be usable. Just because something is a constant does not mean it should automatically be made public and accessible by everything else.
For example why should a constant be made public and then be accessible to all the code in the solution (and maybe even outside the solution) when it is actually only used by a single class?
To aid in encapsulation types and members (including constants) should use the least accessible keyword accessor possible. Developers should not set a public accessor on a type or member simply out of habit.
Fixing the “God Constant Class”
The fix to the “God Constant Class” problem is obviously a move toward greater cohesion and better encapsulation. This means placing constants closer to where they are used and using the most restrictive accessor possible. Once all the constants have been moved then the “God Constant Class” itself can be deleted.
Use of a “God Constant Class” can sometimes be a smell that the application should instead be using configuration settings. The developer may have thought they needed to put all the constants in one place for ease of discoverability/access in case values needed to change in the future. If there is a real case for changing constant values often enough (without changing the code and redeploying the application) then the constant should instead be a configuration setting.