views:

176

answers:

7

A commonly used term is "Magic numbers". As discussed in a related question, this is considered a code smell. I assume the same would go for string constants, although the term "Magic strings" is not in common use.

So to generalize a bit, it would seem that ALL constants should be named, thus making them "unmagical". Or am I missing something? Are there valid uses of unnamed constants?

+5  A: 

I think you can allow yourself some exceptions. I don't treat the following as incorrect:

  • the numbers 0, 1 and -1
  • the empty string
  • unique error messages

Almost (I'm not perfect) everything else I create named constants for.

Edit:

The rationale for not creating names for unique (note unique here) error messages is that I've seen too many projects where this was done but then the wrong constant name was used in the code. This can be very difficult to test for (as som errors are very hard to force) and cause enornous wastes of time for suport staff chasing down the wrong error.

anon
+1 for error messages. If you need to reuse an error message, create it in a reusable method. This way, an error message can't be "fixed" to match situation A (ignoring all the other situations).
Aaron Digulla
+2  A: 

You might want to define it as a constant if

  • Its appearance in code is not self-describing, ie without giving it a name it isn't obvious what it is for, or
  • It is going to be used more than once, or it would make sense for someone else using the code to re-use or modify it.

I don't think there's a reason to define a longish, self-describing string that is used only once as a constant. In some cases it may be better not to, especially where doing so would move it elsewhere in the code from where it is used.

thomasrutter
+2  A: 

A string constant is still a constant, and is a pretty whiffy smell (as a hardcoded string isn't localisable without producing different binaries for each country).

Other than that, I'd mostly agree with Mr. Butterworth... 0, 1 and -1 are too common to create constants for... only name them if you are using them for a specific thing (error cases, etc). The empty string only really needs to be named in special circumstances, which are normally pretty apparent.

Unique error messages I'd disagree with. They should be named and put into a central place so that they can be documented properly. If they are strings, then the above comment about localising them also applies :)

Other than a few exceptions though, name your constants. And please don't name them like:

  • 'ONE'
  • 'TWO'
  • 'EMPTY_STRING'
workmad3
Thanks for mentioning localization. I just spent a week preparing an insane mix of vb, c#, sql, vbscript, html, xml, jquery and js... and of course the UI and error strings for localization. My eyes are bleeding.
Sylverdrag
+1  A: 

7

as in days of the week, but even then I would argue you should give it a constant name.

Dead account
I would agree that even something this trivial should be given a name. The meaning of `DAYS_PER_WEEK` is immediately obvious, while `7` could signify many things. Yes, there are seven days in a week, but seven can signify many other things as well (neutral pH, number of deadly sins, number of dwarf lords given rings of power, etc.). Context should make the meaning clear (unless you are writing some very strange code), but it's much more convenient if the magic number/constant is understandable even before its context is examined.
bcat
A: 

Here's a list of reasons to use them, there are plenty more:

  • Simple unit conversions, like dollars to cents, MB to KB, etc.

  • SQL statements or anything else that will be parsed by machines, not humans. HTML tags fall into this category.

  • Single-use scripts or interactive interpreter sessions.

  • Logging.

  • Anything that is not going to change, ever. Eg. SIDESONATRIANGLE.

postfuturist
+2  A: 

Imagine a codebase in which one particular text string occurs frequently. The string may be some kind of message header, for example, or an XML tag (both of these examples are usually accompanied by another repeated string, representing the message footer or the closing tag). As the codebase grows, you notice the string popping up more and more frequently, until the “duplication” bell goes off. What to do?

It turns out we’ve all been taught the answer: Create a manifest constant (a #define or a static final String or whatever, depending on the language), thereby giving the common string a name, and replace all instances of the string by use of the constant. There’s even a name for this refactoring in Martin Fowler’s catalogue: Replace Magic Number With Symbolic Constant. Duplication sorted, right? Wrong!

The duplication wasn’t in the repeated text of the string’s contents, but in the repeated use of the string. The resulting code still contains the original duplication. Worse, by hiding the string behind a constant, my feeling is that we’ve made the code less readable too.

The right approach -- after using Inline Constant to put the original strings back into the code -- is to look at the context in which each of those strings occurred. Chances are there will be only a small number of kinds of use of the string -- perhaps every time a reply message is constructed, for example. If the message has a header and a footer, for example, I’ll look for ways to create an object that builds the message, with a call-out hook to a function that supplies the message body. Ninety percent of the time that kind of approach works; but on the odd occasion when it isn’t appropriate, I’ll look for ways to encapsulate the string in a simple object, and then push the surrounding code onto that object. In either case the result I’m after is that the string is present inline in the code, but the behavioural context in which that string exists now has a name - hopefully a name that is also meaningful in the application’s domain - and the domain knowledge or policy represented by the associated code is no longer duplicated.

kevinrutherford
A: 

1

Joachim Sauer