2

I am sure this has been asked before, but in this example I am curious as to what usefulness others can see in separating constants to this degree:

public class CoreStringConstants
 {
  // Common strings
  public const string SPACE = " ";
  public const string PERIOD = ".";
  public const string COMMA = ",";
  public const string COLON = ":";
  public const string SEMI_COLON = ";";
  public const string HYPHEN = "-";
  public const string UNDER_SCORE = "_";
  public const string LEFT_BRACKET = "(";
  public const string RIGHT_BRACKET = ")";
    public const string LEFT_SQUARE_BRACKET = "[";
    public const string RIGHT_SQUARE_BRACKET = "]";
    public const string LEFT_CURLY_BRACKET = "{";
    public const string RIGHT_CURLY_BRACKET = "}";
    public const string PIPE = "|";
    public const string CIRCUMFLEX = "^";
    public const string ASTERISK = "*";

... Really?

Is there really any benefit to separating these kinds of string constants from your code?

When is the character used for ASTERISK going to change in the foreseeable lifetime of the application?

dwerner
  • 6,462
  • 4
  • 30
  • 44
  • 1
    Looks like someone got paid by the line... – Kyle Trauberman Nov 17 '10 at 22:50
  • 3
    Reminds me of this http://thedailywtf.com/Articles/Avoiding-Magic-Constants.aspx – Conrad Frix Nov 17 '10 at 22:52
  • Honestly, sincerely, I am not trying to troll-bait. I really just wanted to hear why this could possibly be a good idea. These string constants are used in two places: calling sp's and building messages for the user. – dwerner Nov 17 '10 at 22:53
  • Sadly I've been on a project that had not only a `StringConstants` but a `CharacterConstants` too. Sometimes I was torn between using `CharacterConstants.Period.ToString()` or defining a `StringConstants` version of `Period` for the sake of consistency. Madness. – Ahmad Mageed Nov 17 '10 at 23:04
  • 1
    At least I learned what a circumflex character was. :) – dwerner Nov 17 '10 at 23:22

7 Answers7

4

Actually, I'd consider this a disadvantage. Benefits are dubious to say the least, especially since there is support for pooling string literals anyway, and as a developer I'd have to look up the value of each constant when I encounter code like this for the first time.

Additionally, someone will come up with code that reads

public const string COLON = "*";
Jim Brissom
  • 31,821
  • 4
  • 39
  • 33
  • So how would you deal with the existence of a large number of these 'magic' constants in a project? Search-and-destroy? – dwerner Nov 17 '10 at 22:55
  • @serotonin: Resharper has a very nice "inline" refactoring tool. – StriplingWarrior Nov 17 '10 at 22:59
  • 2
    @serotonin - Just delete the constants. The compiler will helpfully tell you what to do next. :-) – Jeffrey L Whitledge Nov 17 '10 at 23:00
  • You could use a tool like ReSharper to inline the constants and then delete the empty class. (To preempt any snarky comments... Yes, I know that ReSharper is a commercial tool. No, I'm not employed by them. Yes, I'm a fan of ReSharper. Thanks for asking.) – James Kovacs Nov 17 '10 at 23:01
2

In the example you provided, the constants are useless, because as you said, ASTERISK is always going to be *.

It would make a lot more sense to name them after their actual purpose. For example, if you used parentheses to group something in your strings, you could write:

public const string GROUP_START = "(";
public const string GROUP_END = ")";

In this case, it makes sense because the grouping characters could change tomorrow to, say, square brackets.

casablanca
  • 69,683
  • 7
  • 133
  • 150
  • Maybe the original developer figured he could more easily do an automatic rename of "LEFT_BRACKET" to "GROUP_START" if the need arose. Nevertheless, I'd say this is definitely overkill. If you change to a database that doesn't use parentheses in the same way, you're going to have a lot worse issues than changing these constants to worry about. – StriplingWarrior Nov 17 '10 at 23:05
  • 1
    @StriplingWarrior: I guess that depends on the use case. What I had in mind was more along the lines of a string tokenizer - if the format of your string ever changed, it's simpler to change constants than look all over your code to figure out where this specific token is used. – casablanca Nov 17 '10 at 23:09
1

I can't think of a single reason why these string constants need to be defined. Maybe the original author thought that there would be some memory savings by defining the strings once, but the C# compiler is smart enough to intern strings. (e.g. Identical string constants will be output once into the assembly's .DATA section.)

var space1 = " ";
var space2 = " ";
Console.WriteLine(Object.ReferenceEquals(space1, space2));  // Outputs true

So there is honestly no good reason for CoreStringConstants.

James Kovacs
  • 11,549
  • 40
  • 44
1

There are several reasons why someone might want to do this (although there's no need to SHOUT all the constants in this day and age!). However, it is unlikely that there are many good reasons for the particular defines you have listed.

  • Different character encodings may mean that some of the constants could change. Yes, it is "possible" that in a particular character encoding, "asterisk" isn't the same as an ASCII "". Maybe in Chinese a different character may be preferable to "". Is it likely? Well, perhaps not... but having these values in constants will make refactoring easier.

  • Depending on the usage, using a constant allows the character used to be changed throughout the code for easier refactoring. However, in this case I would say those constants are poorly named (e.g. if a curly brace represents the start of a scope, "start scope" would be a better name than "curly brace", allowing you to redefine your system to use (e.g) angle brackets instead of curly brackets to start a scope without the name of the constant becoming confusing)

  • The programmer may have thought that he might refactor to use either strings or chars in future, and by using constants, this choice is easier to refactor later. Of course, one should have more confidence in one's designs than that :-)

  • Perhaps the programmer thinks that constants will cause all the strings to be shared rather than duplicated. String interning usually makes this an unnecessary optimisation.

  • Named constants are often more meaningful than inlined magic constants, and are less prone to typos - this is the only "good" reason I can think of.

Jason Williams
  • 56,972
  • 11
  • 108
  • 137
1

No this doesn't help but this may come from the advice in Steve McConnel's Code Complete section 12.7 Named Constants. Specifically he writes

Avoid literals, even "safe" ones In the following loop what do you think the 12 represents?

Visual Basic Example of Unclear Code

For i = 1 to 12

   profit( i ) = revenue (i ) = expense ( i )

Next

Then he later shows that it would be better to replace 12 with NUM_MONTHS_IN_YEAR or to do from Month_January to Month_Decemener

That's all well and good but the advice fails to make an exception for when you're using a string to that has meaning and aren't magic. For example SQL strings and regular expressions and HTML and CSS strings have meaning and the meaning should be well known to the user.

This kind of thing seems to be a specific case for this question

Community
  • 1
  • 1
Conrad Frix
  • 51,984
  • 12
  • 96
  • 155
0

The only benefit would be that writing CoreStringConstants.SPACE would be a bit clearer and less prone to unnoticed typos than " ". Other than that there is not really any good reason for doing something like that.

As you point out, none of these will ever change, so there is no such reason to centralise the definitions.

Oh, and using all upper case for identifiers is just horrible.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • the only case I see as valid is using `string.Empty` over `""`, which is just a matter of preference, maybe for spaces it's decent too, and newlines `Environment.NewLine` but other than that, Ugh. – bevacqua Nov 17 '10 at 22:57
0

If you have preprocessor directives around them you could specify different sets of them depending on your build. This would be helpful in a large text processing environment which needs to be language agnostic (especially in cases where punctuation marks are used differently).

Chris Pfohl
  • 18,220
  • 9
  • 68
  • 111