In C# Is using private constant strings better than using strings directly in the code?

Question

Consider the following two cases:

• case 1:

  class A
  {
      private const string MyConst="Hello";
  
      internal void CallMe()
      {
          System.Console.WriteLine(MyConst);
      }
  }
  

• Case2:

  class A
  {
      internal void CallMe()
      {
          System.Console.WriteLine("Hello");
      }      
  }
  

Now if the CallMe() method would be called 1000 times, is it a good idea to define it as a constant (which means only one copy of the string per class) or using a plain string would be better. I just checked the MSIL code for both the classes and found that in both cases, .Net compiler is putting the string literal inside WiriteLine(). Then is it an overhead to define the string as a constant?

The main reason I asked this question was because I wanted to understand how CLR works when something is defined as a constant at the class level and when I saw that the MSIL code had const field as well as the string in WriteLine() replaced with the actual string so I was wondering why create constant field if it will be used in only one method. This is just a sample code I wrote to present my doubt.And from the responses I learnt following:

• It's better to use the string directly if it won't be used outside the method
• If it's used throughout the class/classes then declare it as constant at the class level
• Put it in resex if it needs localization

Thanks all for responses/comments.

Answer 1

As covered doing this as a performance thing is not sensible. However doing it for code readability is very sensible. In a non trivial bit of code I would use constants to add syntactic meaning to the variable. Fear the magic string.

class GreetingPrinter
    {
        private const string Greeting = "Hello";

        internal void CallMe()
        {
            Console.WriteLine(Greeting);
        }
    }

The c_str prefix makes my eyes break and adds nothing to the readability of the code.

Answer 2

Case2 is better because the string has no more than the scope it needs and I don't have to chase variables to read your code. Simple. Performance doesn't matter here.

Answer 3

To put it slightly in another direction:

In case of Hello it is nonsense to define a constant and I would go for the second solution. But say you have deoxyribonucleicacid and use it more than once in a class, I would opt for a constant, so you could leverage the IDE / compiler to do your spellchecking. I find that always a nice trick.

Answer 4

I largely agree with Froome's answer and would do something similar in most cases, but I think there are some other things to consider:

For values that only exist in the current method, don't pull the out into a field without thinking. Keep data close to where it used and extract things when it's needed to prevent duplication. the Single Responsibility Principle tells us to keep things together that change for the same reason and to separate things that change for different reasons. This makes it easier to extract behaviors from this class into new classes as the complexity of your system grows.

To me it all depends on what kind of string "Hello" is. If it's a greeting to be output to a user in a GUI it should be pulled out into an external resource. If it's some default value returned by the method used by other methods inside the class, I'd say leave it in the method until you find a compelling reason to move it out of there.

In short, don't assume too much before you know how the value will be used. See where your design takes you and constantly evaluate if you're going in the right direction.

EDIT:

Another thought, if we consider the Open/Closed principle something like this might be appropriate:

public class A
{
    internal void CallMe()
    {
        Console.Out.WriteLine(GetGreeting());
    }

    protected virtual string GetGreeting()
    {
        return "Hello";
    }
}

This allows the behavior to be changed by extending the class but keeping this version closed for modifications.

Again, whether or not this is the way to go depends on what type of system you are building and how it will be used/changed in the future.