6

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.

closed as unclear what you're asking by gnat, durron597, Ixrec, user22815, user53019 Sep 23 '15 at 21:24

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • what do you mean by "better"? – Caleth Sep 23 '15 at 9:26
  • better in terms of memory usage and performance? – GawdePrasad Sep 23 '15 at 9:28
  • 4
    You've already observed that the generated IL is identical, clearly the performance of identical IL is identical. "Better" in programming can also mean easier on you – Caleth Sep 23 '15 at 9:44
  • 2
    Consider this: You continue writing, with "Hello" used all over your class. Next week, you are asked for a Spanish version, so you have to replace "Hello" with "Hola" everywhere, vs once in the constant – Caleth Sep 23 '15 at 10:10
  • 7
    You are micro-optimising. Is your app running too slowly, or using too many resources? If so, profile it, identify the problems and fix them. Until then, write easy-to-read and maintain code. As @Caleth says, the only thing you should be considering at this stage is whether the string is used more than once. If it is, make it a constant. If it needs to support more than one language, make it a resource. To reiterate: absolutely do not try to outsmart the compiler at this sort of level by worrying about speed and memory. – David Arno Sep 23 '15 at 10:13
9

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.

  • I personally like to use such prefixes which helps in understanding what type of variable it is from any part of the code where "c" stands for constant and "str" tells its datatype. – GawdePrasad Sep 23 '15 at 11:14
  • 3
    It is a free world, however you are in a minority. Microsoft's guidelines state "X DO NOT use Hungarian notation." msdn.microsoft.com/en-us/library/ms229045.aspx So something to think about if you are going to have other more standard developers read and understand your code. – Froome Sep 23 '15 at 11:27
  • Thanks for the link, I was not aware that Microsoft has recommended to not use Hungarian notation. – GawdePrasad Sep 23 '15 at 12:44
  • 2
    Nothing is stopping me from using a int c_strMyConst, which is one of the reasons the Hungarian notation shouldn't be used. That is ofc one example that wont hurt you much as you will very fast find out that the string is actually a int. But in a C++ application with dozens of different stringtypes - bstr, cstr, 8bit per char, 16 bit per char, etc., accidentally misnaming a variable instead of relying on the typeinfo from IDE can create hours of hurt. – kat0r Sep 23 '15 at 12:44
  • 2
    The correctness of this answer would become much more evident if you consider that instead of "Hello", the string could be "Χαίρετε". Do you have the slightest clue what Console.WriteLine("Χαίρετε") does? You would have to know Greek to be able to tell that it is a greeting like "Hello". – Mike Nakis Sep 23 '15 at 13:12
7

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.

  • 1
    I don't agree unconditionally but this IS a good point I think many people miss. If anything, I'd prefer putting the constant on the line above the call to the console. That way you get semantics without bloating your class. – sara Sep 23 '15 at 21:50
  • @kai If its usefully namable that's right. Also, for max brownie points, I don't mind tracing into a resx file for this sort of thing. – Nathan Cooper Sep 24 '15 at 2:15
  • If it's worth doing, it's worth doing right, eh? ;) – sara Sep 24 '15 at 7:25
1

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.

  • 2
    And you should immediately update your code to Deoxyribonucleic as the deso- prefix has not been used since... the fifties probably. – Froome Sep 23 '15 at 11:00
1

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.