Class Library of Constants--Best Practice?

Answer 1

The only explanation I can think of is as some sort of message library - but that doesn't hold for 90% of the entries.

For instance this is just silly:

 Public Const cInsert As String = "Insert"

This smells very very bad.

Answer 2

A constant is something that never changes, for example

Public Const NumberOne as Int = 1

So this is my first remark: some of the stuff you summed up isn't really const.

Another downside is that using the const keyword creates a binary depency. This means that you will have to rebuild the assembly which uses your constants.dll. You can't just replace it. This is caused by the way consts work: the compiler replaces the name with the value at compile time.

A solution to this problem is to use ReadOnly instead of Const.

I don't really think it is a good practice to create such a library. I wouldn't allow my team to create one anyway...

Answer 3

Separating out literals from the rest of the code is a good idea.

What's odd is that these should largely be resources rather than constant strings. Then they could be easily localized if needed, or replaced/updated without a recompile of the entire app.

Some of those shoudln't even be resources: cUnderscore for example looks like it's using text to create a visual effect- generally a bad idea.

In your predecessor's defense, I consider this code preferable to finding the same constants scattered throughout the source, as it will make refactoring to resources a little simpler.

Answer 4

This looks like a developer had a coding standard that says: Don't use string literals in code, and dutifully separated out every single constant, whether it made sense or not.

For example, there is probably some element in there where the number 1 was needed in the code and instead of using DefaultNumberOfLineItems or some other descriptive constant, they used NumberOne = 1;

A best practice would be to keep constants descriptive and close to their point of use. There's nothing wrong with a static class of related constants that have some type of meaning and are related to each other.

For example, there is a system that I've worked on that tags attributes with unique keys. These keys are gathered in a static class with descriptive names on the attributes, the actually keys are generated by an automated system

public static class AttributeIDs
{
   public const string Name = "UniqueGeneratedKeyForNameAttribute";
   public const string Description ="UnqiueGeneratedKeyForDescriptionAttribute";
   ... etc.
}

In practice this is used like

MyAccess.GetValueForAttribute(AttributeIDs.Name);

which gets all the related constants together.

Answer 5

There are possible useful scenarios for such a class. Generally, things like "magic number" or "magic strings" are turned in to constants and placed in a static (shared) class. The reason for this is to isolate those "magic" values to a single location and allow them to be referenced by a meaningful name. (Typically used for numeric values.) In the case of string values, it helps ensure that things are referenced by the same value each time. The best example of this is string keys in to your app.config file.

That being said, constants should be used for something that doesn't change (or so rarely changes that it is effectively constant). For the most part, strings that have the potential to change (or need to be localized) should be stored as resources in a .resx file.

Answer 6

There is nothing wrong with having a class library of constants. Constants are a good practice in general. Enums are all over the place in .NET after all, and they are just grouped numeric constants. Having a dependency on an assembly of constants is no different than any other dependency. Understanding the purpose of the constants is more about the app's logic. My guess is that these constants enable verbose logging without filling the app with a bunch of string literals.

Answer 7

Back in the days of coding windows applications in c, there were similar files #included in windows which consisted of long lists of #defines creating constants. Various c applications emulated this approach in their own files. The "class" seems to be a "transliteration" of this "c-ism". The fundamental principle of Object Oriented Design is to mix code and data into related functional units: objects. As jfullerton wrote:

From a programming point of view, object-orientation involves program objects, encapsulation, inheritance, and polymorphism. The conceptual objects are modeled in the program code. Encapsulation keeps an object's data and methods that use the data together as part of the object.

So clearly, this constant list does not conform to OO practices but is a throw back to the old days.

To answer your questions:

1. -- This class holds constants, that is it
2. -- The old developer probably did this because that was what he was used to doing
3. -- It's not current best practices.

Naturally, if this is part of your application, you can't just throw this away. Rather, this is something to refactor over time, assuming you use current best practices of Test Driven Development and Refactoring

Answer 8

It really looks like a naive implementation of a string table.

The values are no magic strings and "easy" system-wide changes. I would argue that a resources file is easier to both implement and maintain.

Is your colleague's implementation a best practice? I'd say no. Using string tables, it is, especially if you need internationalization in your app.

Answer 9

Putting string constants into a separate class is a best practice in many situations, however, this is a poor example. A better way would be to create a StringConstants namespace and then organize the strings so that related string constants are organized into separate classes. This is just a poor implementation of a good idea.

If globalization is a concern, then (as others have pointed out), strings should be kept in resource files.