Member variable decoration

[Michi Henning]

The style guides says:

    Variable names are all lowercase, with underscores between words. Class member variables follow this convention. For
    instance: my_exciting_local_variable, my_exciting_member_variable.

This was changed from the original text in the Google style guide:

    Variable names are all lowercase, with underscores between words. Class member variables have trailing underscores. For
    instance: my_exciting_local_variable, my_exciting_member_variable_.

I take it that the intent is to *forbid* decoration of member variables rather than just making it optional.

However, there is no rationale to explain why not decorating the members is better.

Some thoughts on this:

By all means, if I have a class with only one or two members (such as pimpl which always only has a single data member), decorating the member name is unnecessary.

For classes with several member variables that do complex things, I don't think it's all that clear-cut.

Consider seeing "x = val;" somewhere in the body of a member function. What kind of thing could that "x" be that's assigned to here? I can think of the following (not sure whether that's a complete list).

- local variable
- parameter
- member variable
- member variable in a direct or indirect base class
- file static variable
- namespace variable in a direct or indirect enclosing namespace
- namespace variable in an anonymous namespace in the same source or header file
- extern variable
- global variable

Why is it desirable to force member variables to look exactly like every other variable? Has anyone ever noticed the similarity of member variables to global variables? While I'm inside the body of a method, I can just assign to member variables without having to declare them. They just "float around", much like globals do.

When I write code, I find it helpful to distinguish member and non-member variables. I find it also helpful when reading my code or someone else's code later. If I see "m_var" (or "var_"), I know without having to check anywhere at all that a member variable is being assigned. This reduces the amount of state I have to hold in my head, and saves me having to look for the definition of the variable in a header file. This is particularly true when I need to work with code written by someone else that I'm not familiar with.

Now, we may disagree on where the trade-off point should be between decorating and not decorating. But why should it be *forbidden* for me to this?

The member variable decorating convention has been with us for a good twenty years. It is recommended by the following style guidesj (and probably many others):

- Google
- Geosoft
- possibility.com
- ros.org
- GNU
- Microsoft
- C++ Coding Standards (by Sutter and Alexandrescu)

Just because something is old doesn't make it right. But I'm truly wondering whether disallowing the decoration is the right thing to do, especially when considering that most C++ programmers will be used to it.

There is also member variable initialization, for which the following has become very much idiomatic:

MyClass::MyClass(const string& path, int size, bool try_again) :
    m_path(path),
    m_size(size),
    m_try_again(try_again)

If I can't use decorations, I have to artificially invent parameter or member variable names that differ, just to avoid the namespace collision. If nothing else, that's annoying.

I would much prefer for the style guide to at least permit the decoration. Whether in the form of "m_var" or "var_" is neither here nor there to me. (They both seem to be used with about the same frequency.)

[Jussi Pakkanen (jpakkane)]

The main point of decorating class variables is that "they can be easily separated from local variables". This description itself tells that what decoration is doing is treating a symptom, not the cause. If you can't easily tell the difference between local and class variables, the method is _badly designed_. To quote Clean Code:

Functions should do one thing. They should do it well. They should do it only.

And:

Functions need to be small.
They need to be even smaller.
They should probably be smaller than that.

In C it is hard to have small functions because of low level language primitives. In C++ you can have very readable and expressive code so that most functions should not exceed 15 lines or so. When your code is simple, the difference of variable type becomes obvious. That is what we should be focusing on: improving code quality rather than inventing better and better crutches.

> Why is it desirable to force member variables to look exactly like every other variable?
> Has anyone ever noticed the similarity of member variables to global variables?

If you have enough global variables to confuse them with local variables the problem is unambiguous: you have too many global variables and should get rid of them as soon as possible. Most applications should have zero. If you want to separate globals from instance variables then decorate the globals (maybe put them in a single global struct or something). That makes more sense because dealing with global variables is an exceptional case and should always be done with caution. It is better to emphasize the exceptional rather than the common.

> The member variable decorating convention has been with us for a good twenty years.
> It is recommended by the following style guidesj (and probably many others):

On the other hand the style guide of Java, which was designed after C++ by people who used C++ for the implementation and it specifically forbids decorations.

> There is also member variable initialization, for which the following has become very much idiomatic:
>
> MyClass::MyClass(const string& path, int size, bool try_again) :
> m_path(path),
> m_size(size),
> m_try_again(try_again)
>
> If I can't use decorations, I have to artificially invent parameter or member variable names that differ,
> just to avoid the namespace collision. If nothing else, that's annoying.

This is not necessary. You can use the same name, like this:

class Foo {
private:
  int x;

public:
  Foo(int x) : x(x) {}
};

Compiles, works, does exactly what you'd expect. If you absolutely, positively, have to have decoration, put it in the constructor argument because then it is isolated to one line and does not leak to other parts of the code.

Some random thoughts:

If you must have decoration, use the suffix rather than a prefix. The reason for this is that the prefix makes code completion more tedious. (i.e. to complete variable myVarName_ you can just type myv + ctrl+space, as opposed to m_myv + ctrl+space)

If you use a pimpl, all variables should be put in it and the variable name p should not be decorated. The reason being that p works as a decoration of sorts and adding more is just noise. Cont...

[Michi Henning (michihenning)]

I agree with much of what you say. Short functions, clarity, no global variables, etc.

Often, I didn't write the code, but someone else did who wasn't sticking to these principles. I *frequently* see functions that extend over several hundred lines and classes with a dozen or so member variables. (Have a look at the Nux code. It has if-then-else statements that extend over several hundred lines, never mind functions…)

Basically, to me, the decorator does no harm, but helps me keep my mind organized a little more easily. So, the question is whether it should be mandatory to *not* have a decorator, or whether it should optionally be allowed. (Ideally, there should be as few optional style recommendations as possible. But, I realise that this is close to religious territory, and we can't please everyone.)

class Foo {
private:
  int x;

public:
  Foo(int x) : x(x) {}
};

Thanks for that. It never occurred to me that this would actually parse correctly. Sheesh, after all this time, I still don't know this bloody language completely :-(

I agree about the pimpl. A simple "p" is just as effective as a "p_" or "m_p".

My personal preference is for "var_" rather than "m_var", but I can live with either. I hear you on the postfix being more convenient for code completion.

[Michael Zanetti (mzanetti)]

> class Foo {
> private:
> int x;
>
> public:
> Foo(int x) : x(x) {}
> };

While this works, getter functions don't:

int foo()
{
  return foo;
}

I guess you could use "return this->foo;" here, but I for one prefer the m_ prefix.

Regarding the code completion, I personally like the fact that hitting "m_ + ctrl + space" gives you a nice list of member variables, leaving everything else out.

In the Qt world the m_ prefix is quite common. For private implementations, Qt offers some macros that play well with Qt's way of doing private implementations. For example Q_D(), which creates a pointer to the pimpl called "d".

IMO for Qt code we should follow what upstream Qt and the rest of the Qt world does. It plays very well with the API's that Qt is offering and makes the code readable for people knowing Qt. On the other hand I would not suggest to force Qt's standards on non-Qt code. Exactly this is the biggest issue about the whole thing. We try to find one single style guide for different type of projects while I think it should be adjusted to the used toolkit and/or surrounding way of doing things.

Just my 2 cents.

[Michi Henning (michihenning)]

> IMO for Qt code we should follow what upstream Qt and the rest of the Qt world does.
> It plays very well with the API's that Qt is offering and makes the code readable for
> people knowing Qt. On the other hand I would not suggest to force Qt's standards
> on non-Qt code. Exactly this is the biggest issue about the whole thing. We try to
> find one single style guide for different type of projects while I think it should
> be adjusted to the used toolkit and/or surrounding way of doing things.

That's a really important point. If the code has to fit into an already-existing ecosystem, it makes sense for the code to follow the expected style for that ecosystem, rather than trying to educate the rest of the world as to the one-and-only correct way of doing things.

So, for the Qt/QML code to use an m_ prefix sounds entirely reasonable to me.

And I think there may be other legitimate reasons for a particular project to override the style guide for some particular point. (I would expect tech leads to not override the style guide without good reason.) But if we don't make room for some judgement calls, the style guide will just end up back-firing on us and cause more damage than good.

A case in point: the style guide mandates neither K&R or Allman bracket style. I'm happy with either. But I'm *not* happy with having two fundamentally different layout styles in the same project. So, we are using Allman. If another project thinks that K&R is the best thing since sliced bread, that's fine with me, and the project is welcome to make its own call on this.

[Jussi Pakkanen (jpakkane)]

> I *frequently* see functions that extend over several hundred lines and classes with a
> dozen or so member variables. (Have a look at the Nux code. It has if-then-else statements
> that extend over several hundred lines, never mind functions…)

If you have coders that don't care enough to make their functions short, they probably won't care about what the style guide says about variable naming. Unfortunately. :(

That being said, using decoration for this kind of code as a tool to clean it up is perfectly fine. It's just that new code should be written and peer-reviewed well enough so it does not need decorations.

For Qt code using Qt style is totally cool.

[Daniel d'Andrada (dandrader)]

I prefer to have a decoration for member variables (due to some reasons already exposed).
That said, I prefer shorter ones (a single "_" char) as opposed to the "m_" prefix.

[Michi Henning (michihenning)]

We have preferences for no decoration, decoration with a trailing underscore, and decoration with an m_ prefix. Rather than mandating one of these, I would like to reword the relevant paragraph as follows:

    Variable names are all lowercase, with underscores between words. For
    instance: my_exciting_local_variable.

    For trivial classes with few member variables, no decoration is required, that is, class
    member variables can use the same syntax as a local variable: my_class_variable.

    For more complex classes, consider decorating class variables with a trailing underscore:
    my_class_variable_. This can improve code readibility for classes with more complex state.

    Alternatively, class member variables can use an m_ prefix as decoration: m_my_class_variable.

    As always, try to be consistent. In particular, if a class needs to work with or extend an existing
    framework (such as Qt), follow the established naming convention of the framework rather
    than adding a new convention.

[Jussi Pakkanen (jpakkane)]

Since style rules are about consistency, we should state that using the m_ prefix is only allowed when using frameworks that recommend it (e.g. Qt).

[Michi Henning (michihenning)]

> Since style rules are about consistency, we should state that using the m_ prefix is
> only allowed when using frameworks that > recommend it (e.g. Qt).

I can live with that. Michael, your opinion? If we can agree on using a trailing underscore and m_ only for Qt or other frameworks that use this convention already, I think we have a quorum :-)