Cppcoreguidelines: Discussing the `_` style mentioned briefly in `NL.5` (contravertial?)

[galik]

I understand that when it comes to guidelines regarding formatting style people get very territorial and I am not sure its a great idea to try to impose arbitrary rules on people beyond coding in a way that is clear and readable to others. But if there are going to be style guidelines then I think it would be good if an argument can be made why someone considers them either beneficial or potentially detrimental with respect to safety/efficiency/maintainability etc.

So with that in mind I thought I would add my thoughts on one style that seems to be gaining traction but that I feel can actually be a source of potentially hard to spot bugs. You may disagree. I could be over-estimating that potential. But here at least are my thoughts on the issue.

NL.5 Don't encode type information in names

... <snip> ...

Note

Some styles distinguishes members from local variable, and/or from global variable.

struct S {
    int m_;
    S(int m) :m_{abs(m)} { }
};

This is not evil.

Although I agree that "styles that distinguish between members and locals" are not _inherently_ evil I do feel the one presented in the example may well be. It's very easy to forget to add the _ or to add the _ when you meant to omit it. Also people differ on whether they add it to the member or the parameter increasing the potential for confusion.

One thing to remember is that the language itself contains a perfectly good means to differentiate the two which, to my mind, is both _clearer_ and _safer_ than the given method:

struct S {
    int m;

    void func(int m) { this->m = m; } // hard to get wrong 
};

Using this->m = m is very explicit and impossible to confuse with m = this->m or even m = m (which is obviously wrong). But it is very easy to confuse m_ = m with m = m_ because you are already expecting them to look the same.

When you use the _same name_ doing m = m; rather than this->m = m; points itself out as an obvious mistake. However when one of the variables has a subtle, easy to miss _ then confusion between m_ = m and m = m_ is much easier.

struct S {
    int m_;

    void func(int m) { m = m_; } // whoops wrong direction
};

And remember that _some_ programmers give the member variables the underscore while others give it to the parameter leading to potential for mistakes when a programmer who is used to adding _ to the parameter edits code by a programmer who added _ to the member and vice-versa.

I believe that is it both _safer_ and _clearer_ to _either_ always use the _same name_ for parameters and members or else use a naming scheme that _visually differentiates_ the two. The big problem with _ is that it is trying to pretend the variables have the same name _when they don't_ and succeeds well enough that it becomes easy to confuse the two.

If you actually use the same name then you are forced to use the clear and obvious language feature that differentiates between the two and which _also_ expresses intent:

    this->m = m; // says exactly what you mean, very hard to mess up

And going back to the original example using the constructor, there is always the possibility of this happening:

struct S {
    int m_;
    S(int m): m_{abs(m_)} { } // whoops wrong variable
};

But:

struct S {
    int m;
    S(int m): m{abs(m)} { } // can't go wrong, compiler guarantees.
};

The language itself makes the scoping unambiguous here and I feel the safest way is to use the same parameter names as the member names. Doing that makes subtle errors much harder to perpetrate. So the _language itself_ has _already fixed_ the problem that using _ is trying to solve and imo the language solved it in a safer way.

To summarize I think the _ is very easy to get wrong compared to either using the _same name_ for members and parameters or else use something more _visually obvious_ like m_variable = variable; I feel that is much harder to get wrong than variable_ = variable;. If you are going to use a technique designed to make the members look _visually the same_ as the parameters then i feel it is safer to actually make them the same because C++ is designed to deal with it. So my preference is to either make them the same or use something that makes them look _clearly different_.

[Pazns]

I think maybe an issue with _ is that it means nothing at all in the first place.
Weren't variables invented to give a readable and abstract meaning to what was only computing operations ?

When you are writing :
void func( int m) { m = m ; }
you want to replace a (internal) m value with a value that is new, so maybe you could have something like :
void func( int new_m) { m = new_m ; }

Besides this, I can't understand why internal members are so often isolated with m_/_ prefixes.
This is an object, what he knows first is himself, so why isolate him from the rest of the world when you should isolate the rest of the world from him ? External and input variables should be prefixed instead, if prefixes are needed.

[Amaroker]

I can't understand why internal members are so often isolated with m_ / _ prefixes.

In my experience, it makes it easier to reason about code.

Let's say you use the prefix "m_" for (and only for) member variables. As a result:

• Local variables will never hide member variables.
• It's easy to tell if a variable is a member without looking at the rest of the code.
• It's possible to search for member variables.

Likewise, you can use the prefix "g_" for global variables.

NL.5 says "Hungarian notation is evil", but I do believe that it can be helpful. Consider:

if( x == y )

Is the above a good comparison? Maybe x and y are float numbers. It's impossible to tell by this example.

if( nX == nY )

Thanks to the notation, where prefix n means integer type, we know the intention is to compare integers.

Another example:
If you want to call a method on a variable, do you write "." or "->"?
Using a pointer notation, where prefix p means pointer type, makes it easy:

x.foo();
pX->foo();

[Amaroker]

Regarding the example using the constructor, I agree that the prefix "m_" is more distinct than a trailing underscore.

In the example, the variable is called "m", which is unfortunate since that can be a shortening of the word "member" (when it's written with the postfix underscore, it's looks just like a stand-alone prefix).
I suggest the variable in the example should be changed to something other than "m".

Anyway, it's hard to get it wrong if you declare the parameter as const:

S(const int m) :m_{abs(m)} {}

[egeerardyn]

Likewise, you can use the prefix "g_" for global variables.

I think at that point you are using what I would call a pseudo-namespace, while it would be just as easy to introduce a namespace global to serve the same purpose.
Why do something by arbitrary convention when C++ has the features to do it in a non-ambiguous way?

NL.5 says "Hungarian notation is evil", but I do believe that it can be helpful. Consider:

if( x == y )

Is the above a good comparison? Maybe x and y are float numbers. It's impossible to tell by this > example.

if( nX == nY )

Thanks to the notation, where prefix n means integer type, we know the intention is to compare integers.

I believe that using Type Hungarian Notation (i.e. what you describe) is indeed evil. You are encoding the type in the variable name, which is redundant information. Redundant information tends to go stale and then you have a big problem. Then that snippet may look fine while it is not, which is IMHO even more harmful.

Joel Spolsky made a reasonable argument about Apps Hungarian Notation, i.e. use Hungarian Notation to encode some semantics of the variable (he exemplified this with "safe" versus "unsafe" strings in the context of HTML/SQL/... injection).
However, I believe that in manycases, you could encode such information in the type system even and then have the compiler do the dirty work of enforcing correctness of your code.

Moreover, I think that with modern IDEs, Type Hungarian Notation is never the good solution. For more basic IDEs or text editors (such as vim, atom, sublime, emacs, ...), I think it could maybe help you in some cases, but I don't think it's a good modern practice (which is what we should be advocating here).

[Amaroker]

@egeerardyn, Agreed, a separate namespace for global variables is even better.

Regarding Hungarian notation, I just think calling it evil is a bit harsh. Indeed, it is redundant, and as such it requires discipline to rename the variable if the type is changed.

In the (x == y) example, let's say it compares a float with an integer. That was probably not the intent. But the compiler doesn't complain (at least not the ones that I've tested). It might go unnoticed for a long time, but sooner or later it may cause unexpected behaviour.

Using Hungarian notation is not a perfect solution, but can sometimes help detect errors early. Already at the time of writing the if-statement, it would have been obvious that different types are being compared.

[Pazns]

it requires discipline to rename the variable if the type is changed.

But why do you encode the implementation in the name of your variable ?
It is non-sense, the name of a variable should be unaware of how she is representing the abstract concept you are programming.

Plus, a programmer should always watch the type of a variable he is using.
If you have to remember to prefix each of your vars, maybe you could remember to always watch the type instead.

[Amaroker]

But why do you encode the implementation in the name of your variable ?

In this case to avoid unintended mixing of POD types where implicit conversions will occur. It's all about the awareness of the human reading the code. A member prefix/postfix is also an implementation encoding.

You can reason about a code snippet, without having to find the declaration of the variables (if it's a member variable, you may need to find and open the correct file).

Except for the pointer notation, Hungarian notation can only be helpful for variables in a non-generic context and not when declared with auto.

Like egeerardyn said, a modern IDE can aid to quickly identify the types. In that environment, the notation is less useful.

[Pazns]

In a real-life-case of "Big Ball of Mud" program, this is a boring thing to endure as a maintenance developer. Hungarian prefixes are getting in the way everywhere, hinting at types in a wrong/unreadable/obsolete/redundant manner.

A disciplined developer will surely benefit from this practice, you are right. But it requires also a disciplined developer to write and maintain it. And maybe, I think, such developer doesn't need it in the first place.

As @egeerardyn said :

Why do something by arbitrary convention when C++ has the features to do it in a non-ambiguous way?

Personal style is a wonderful thing for what is not in the language itself, but we can say developers _should_/_could_ (and as you point, not _must_) avoid superfluous technical mimicry.

[Amaroker]

"we can say developers should/could (and as you point, not must) avoid superfluous technical mimicry. "

Yes, that we can say instead of condemning notations as evil. I'm primarily against the wording. If notations are commonly used in a wrong manner, or they make variables unreadable, these are valid arguments against them.

Well then, for what reason is the underscore notation for member variables an exception? It's also redundant, and as the original poster said "very easy to get wrong".

[egeerardyn]

Regarding Hungarian notation, I just think calling it evil is a bit harsh. > Indeed, it is redundant, and as such it requires discipline to rename the variable if the type is changed.

I agree that it might be a bit harsh to call HN plain out evil. In some cases it is a necessary evil (or the best of two evils), so perhaps it should be worded less strongly.

Well then, for what reason is the underscore notation for member variables an exception? It's also redundant, and as the original poster said "very easy to get wrong".

There are subtle differences, but I really agree with the point of @galik and you that it can be confusing. So I also prefer some of the alternatives that have been discussed above (e.g. this->m = new_m;, this->m = m, ...).

Personal style is a wonderful thing for what is not in the language itself, but we can say developers _should/could_ (and as you point, not _must_) avoid superfluous technical mimicry.

I wholeheartedly agree. Guidelines are guidelines, but if you have good reasons to disregard the guidelines (if that yields better code in your case), by all means, do.

[MikeGitb]

Marking private member variables
I am a fan of marking member variables, but personally, I find that writing this-> everywhere adds too much noise. Prefixing the variables with m_ or just _ usually does the job for me.

Hungarian notation
Regarding hungarian notation, I agree with @egeerardyn that it depends on whether we are talking about the systems or the apps flavor of it.

I personally consider systems hungarian (where the actual type is encoded in the name) in modern c++ pretty useless:

• For one, there are imho too many base types (int, short, long, signed/unsigned float/int, raw pointer ...), typedefs (uint8_t, size_t, DWORD, ...) and "smart wrapper" (ùnique_ptr,optional` ...) with platform dependent mappings to allow for a simple and consistent naming style.
• Second, most of my variables aren't fundamental types anyway but clases or enums, for which there is no reasonable hungarian naming scheme anyway.
• Third, what fundamental types I have are usually either function local, or enclosed in a class, so it is relatively easy to find the declaration when needed.

Add to the mix the fact, that you can't rely on variable names being updated, when the type changes and as a result, there are very few instances, where hungarian notation provides useful, reliable information. In particular, I prefer tools warning me about things like floating point comparison or narrowing conversion (which are the only two instances I can think about right now, where hungarian notation might prevent bugs).

Although I've never adhered to a strict apps hungarian notation there are certainly quite a lot of variable names that are written in that flavor like e.g.

• it_ denoting an iterator
• m_ denoting a member variable
• ptr for pointers
• t for temporaries
• d for delta/difference

So I'd say, a naming scheme is most useful, when it adds information to they type instead of repeating it.

Another example for this that was more relevant in the past is to encode whether a const char* represents a zero terminated ASCII string, a zero terminated utf-8 String, an array of numbers or just a raw byte array. However, thanks to the gsl types, I hope to see this much less in the future.

[BjarneStroustrup]

It would take a lot to make a significant change here. Just about everybody has strong opinions on naming. Also, libraries, applications, and organizations have well established conventions that a programmer cannot mess with. What we have are comments aimed at helping novices, avoid the worst excesses, and for the rare programmer with a free choice. We are not going to recommend a specific for of member/scope/whatever notation. "Not evil" is a weak comment meant to avoid condemnation based on other rules, such as NL.5 itself. And, yes, we do consider Hungarian "evil" in the context of modern C++: It makes good programmers write ugly and hard-to-maintain code.

[Amaroker]

Ugly is an aesthetic opinion, but I do agree in cases where the notation is more than a couple of letters. That's to overdo it.

Anyway, the default should be to use auto, which is enforced by:

ES.11: Use auto to avoid redundant repetition of type names

However, in a few cases you may have reason to explicitly write the type (for example int instead of auto) to express intent.

P.3: Express intent
Unless the intent of some code is stated (e.g., in names or comments), it is impossible to tell whether the code does what it is supposed to do.

In my previous code snippet, it's impossible to tell without a name or comment that reveals the type.

I don't think it's necessarily hard to maintain. When explicitly writing int, a tool can easily enforce that the variable starts with 'n' followed by a capital letter. If the type and notation doesn't match, the tool warns about it. Then it's just a matter of replacing the name, which can also be done automatically by a tool.

Don't get me wrong. I'm not saying that should be a general recommendation, but I don't think it's all evil.

[Pazns]

@Amaroker :

In my previous code snippet, it's impossible to tell without a name or comment that reveals the type.

I'm lost, what snippet ?

[Amaroker]

@Pazns This one:
if( x == y )

Is it supposed to compare floating numbers?
Is it supposed to do an implicit conversion?
Maybe that was the original intent. Or maybe not.

A notation for POD types is one way to make the intent clearly expressed and helps avoid mixing such types by mistake. I don't see why notations are particularly evil if they're enforced by a tool. An extra character doesn't hurt readability much in my opinion, though there might be better ways.

[Pazns]

@Amaroker :
your example is too abstract, I think, to prove your point.
if( x == y ) means nothing without any context.

Is this coordX and coordY in a spatial context ? leftOperand and rightOperand in a generic arithmetic thing ?

If you are doing something precisely on a floating point number, then maybe you should write floatingA and floatingB, because you are working with the abstract concept of a floating point number.
If you updates your program and make float be a double or even a long double, you still have a floating point number. You shouldn't program too implementation-specifically.

[BjarneStroustrup]

Use longer names where appropriate; see NL.7. In a sufficiently small function and used conventionally x and y are quite appropriate.

[galik]

For documentation purposes here is a relevant example of the original point.
https://stackoverflow.com/questions/46033643/why-gcc-does-not-report-warning-about-uninitialized-value
It's not a good code example tbh but it does illustrate how easy it is to introduce hard to spot bugs when you chose very similar (but not the same) names (eg by adding just an a _). If you use the same names, the compiler does the correct thing and you are always using a properly initialized value to initialize the uninitialzed members. But when the names differ you have to get it right manually. In that case overly similar names invites human error.