Ban string literals from your code!

In my opinion there are two “types” of string literals you’ll find in most of the code:

Strings to be displayed to the user

What I am alluding to is code like the following:

MessageBox.Show("There has been an error processing your query");

Why is this code bad?

Leaving out the background of User Experience (the user is not informed about the reason), there stay two main reasons in my opinion:

  • Localizability: String literals specified in code can not easily be translated in order to provide UIs with multi-language support.
  • “Overloading” of the code: Though not getting visible in this example, it can be a problem when having lots of messages which are longer than that. The real logic the code performs gets more obscure with each irrelevant information it contains.

Having the messages distributed all over the code makes them also difficult to find. So (spell-)checking is hindered.

How to solve?

The solution is quite simple (and you already may know): Use Resources.

When using VS there will be generated a class for easy accessing it automatically. Our code could now look like:

MessageBox.Show(MessageResource.ErrorProcessingQuery);

Much cleaner, isn’t it?

In addition, now even people having not the skills to understand (C#)-Code can maintain the messages in your application by using a resource editor.

Strings affecting the program logic

What am I meaning by that?

There are some well known scenarios:

  • Accessing a Database using DbCommand and DbDataReader: Both the SQL-Query and the column names are often specified directly within the code.
using (SqlCommand command = connection.CreateCommand("SELECT * FROM CUSTOMERS"))
{
    SqlDataReader reader = command.ExecuteReader();
    while (reader.Read())
    {
        Console.WriteLine(reader["FirstName"]);
    }
}

  • Parsing XML-Files: Here string literals are identify elements, attributes and so one
  • Reflection or “Pseudoreflection” where strings represent property names or member names in general
  • Regex patterns

Again the two questions:

Why is this code bad?

Do some refactoring including member renaming. Then you’ll know why at least the first one definitely is ;-)

Although the other ones are not really evil, it’s nevertheless not ideal. If there occur renamings in the schema, you’ll have to change code.

And, almost the worst thing: You (or the user of your library) can easy misspell a name of a property or violate specifications without noticing at compile time. There is absolutely no check until the code is executed.

How to make it better?

Of course the possibility to use resources as shown above is still given.

However, it might be better (it’s totally depending of the actual situation) to use Configuration Files in the first two scenarious (especially in the first) and the fourth one. That would allow renaming of Columns / Elements or weakening a to strict pattern even without recompiling the application.

Now what about the third case? Well, its “solution” is a little bit more tricky. In most cases it will connected with some restructuring. In short: You’ll have to find another way to specify what you want. Some approaches that may help:

  • Provide an enumeration with all possible values
  • Follow suit the DataLoadOptions.LoadWith method and allow a construct like: foo.DoSomething(bar => bar.Property); Magic Strings O Magic Strings shows how.
  • Get inspired by Color and use an “enumeration-like” construct (Color contains static members each representing a specific color e.g. AliceBlue, Red, Green)
  • Design classes that allow modeling statements you previously specified as string (instead of a parser) – of course only, if the parser was used code-intern only (I know: a little bit uncommon).

If nothing applies…

… there last still one thing you should always avoid to do:

Do not specify string literals within members.

In most (all?) of the remaining cases it’s the best to introduce constants at the beginning of the class (or where you like to place them;-)) representing the literal so you don’t have to browse all members if something changes unexpectedly.

What do you think?

Do you agree with me?

Do you have you any other ways to eliminate string literals from code?

DotNetKicks Image
About these ads
Follow

Get every new post delivered to your Inbox.

%d bloggers like this: