Should single-use values be inline, function-level

Question

I have a function that performs a few string comparisons based on an argument. The strings that are being compared against are not used elsewhere. My instinct is to declare all of the strings as consts at the beginning of the function. However, they could just be inline, or declared on the class level. What is preferred?

Here is the gist of the function:

void MyType::parse(const wstring& input)
{
    if (input == value1) { do1; }
    else if (input == value2) { do2; }
}

Possible options for the values:

A. Inline values:

if (input == L"foo") { do1; }

B. Function-level values:

void MyType::parse(const wstring& input)
{
    const wstring foo = L"foo";    
    if (input == foo) { do1; }  
    ...
}

C. Class-level static constants:

.h

class MyType 
{
private:
    static const std::wstring kFoo;
}

.cpp

const wstring MyType::kFoo(L"foo");
...
void MyType::parse(const wstring& input)
{
    if (input == kFoo) { do1; }  
    ...
}

There are probably other options as well. Now, opinions differ as to readability, so while those are important, it's impossible to have a definite answer about that. So, when I ask, "which is preferred?" I'm asking about which performs best and has the lowest complexity.

Answer 1

My personal preference:

Keep the literal as close to the point of use as possible - so options A or B, but not C.

To choose between A an B, ask yourself "Does the literal itself make sense to someone else reading this code?". If it does, go for A and the code is still self-documenting. If it doesn't, option B gives you the opportunity to provide a meaningful name to the literal.

Examples:

// option A
void MyType::parse(const wstring& input)
{
    if (input == L"QUIT") { quit(); }
    else if (input == L"CONTINUE") { read_next(); }
}

// option B
void MyType::parse(const wstring& input)
{
    static const wstring quit_command = L"*34!";
    static const wstring continue_command = L"*17!";

    if (input == quit_command) { quit(); }
    else if (input == continue_command) { read_next(); }
}

Answer 2

What do you prefer?

They're not all equivalent of course.

If you give them (named) namespace or global scope, they can get external visibility, meaning you can define them in a separate TU and even change their definition without recompiling (just linking). If that TU is in a dynamic library that linking might be at runtime.

Also, function locals are usually not separately documented. However if these values have significant meaning, you might want to document them. If you don't wish to imply external linkage, make them file-static, e.g.:

namespace /*local to TU*/ {
    /** @brief the file pattern is used when ... 
     */
    constexpr char const* file_pattern = "......";
}

That way, your class declaration doesn't leak implementation details and doesn't need to change if those details change.

So, it's up to you. But consider your needs for testing, maintainability and documentation.

Answer 3

The question you have to ask is: do you see this string being changed/modified in the future? If so, inline will not work. If you know this string will never be changed, received through a get() function, or modified, then I would say inline is best since you do not have to declare space in memory to hold the variable (and save a line of code).

Answer 4

I personally would go with the last variant. You have one single point of change if you require your "Magic String" to be modified, which is always a good idea. Even if you only use the string once, I would suggest that you still have one constant somewhere, otherwise you will do it the one way for this scenario, but the other way in another, which is inconsistent.

Just my 2 ct.

Answer 5

Generally, you should refer to your employer's coding standard.

If that does not explain which to use, ask your team lead.

If he/she does not care, your instincts are fine.

My experience has been varied ... I prefer the const std::string defined close to the first time used.

---

Edit: (some now missing comment apparently thought the above was incomplete)

Should single-use values be

• inline function-level const variables,

• class-level static const variables

As I previously stated;

I prefer the single-use value as close to the first time use as possible. and thus not in the class-level constants (neither static nor otherwise)

I generally prefer them on their own line, so perhaps this means not in-line, and not anonymous. I suppose this is related to the idea of "no magic numbers in your code." (even though this is not a number.)