5

Is this valid C++, assuming I want to copy the argument variable to the member variable:

struct Struct {
  Struct(const T& value) : value(value) {}
  T value;
};

(Update: It works in Visual Studio, but still that might be compiler dependent) (Expected question: Why do you need this? Answer: Macro-making purposes)

Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
Viktor Sehr
  • 12,825
  • 5
  • 58
  • 90

7 Answers7

4

This is indeed valid code and like the rest of the answers I will warn you that this should be used very carefully since it can be confusing and would probably lead to hard to maintain code.

So why does this work? If we consider your constructor:

Struct(const T& value) : value(value) {}
                         ^     ^
                         1     2    

1 and 2 are evaluated in different scopes. So we need to look at the draft C++ standard section 12.6.2 Initializing bases and members and look at some grammar:

ctor-initializer:
    : mem-initializer-list 
mem-initializer-list:
    mem-initializer ...opt
    mem-initializer , mem-initializer-list ...opt
mem-initializer:
    mem-initializer-id ( expression-listopt )
    mem-initializer-id braced-init-list

After digesting this we see that 1 is really a mem-initializer-id and 2 is a expression-listopt and we can go to paragraph 2 and 12 respectively for each of these. Paragraph 2 says:

In a mem-initializer-id an initial unqualified identifier is looked up in the scope of the constructor’s class and, if not found in that scope, it is looked up in the scope containing the constructor’s definition. [...]

So 1 will be looked up in the class first while we can see from paragraph 12 which says:

Names in the expression-list or braced-init-list of a mem-initializer are evaluated in the scope of the constructor for which the mem-initializer is specified.

2 will be looked up in the scope of the constructor. So 1 will find the member variable first and stop looking while 2 will look in the constructor and find the parameter. This also means that if you wanted to refer to a member variable in an expression-list you would have to use this->.

Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
3

It is perfectly valid, standard-conformant code, beyond any doubt.

Struct(const T& value) : value(value) {}
                               ^^^^^ this is argument
                         ^^^^^ this is the member

Now the question is: is this good practice? In my opinion, NO. I prefer my data-members to follow different yet consistent naming convention, such as data-members always start with _. So I would prefer this:

Struct(const T& value) : _value(value) {}

where _value is data member. You could follow any naming convention — just make sure that you're consistent.

Note that in your code variable, functions, classes or any identifier should not start with double underscore such as __value, or single underscore followed by upper case letter such as _Value — such names are reserved for implementations.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • Aren't names prefixed with underscores reserved for the implementation (->UB)? –  Dec 16 '13 at 16:05
  • 2
    @H2CO3: No. If you use *single* underscore followed by *lower* case letter, then it is perfectly fine. `Names such as __value` or `_Value` not allowed, though! – Nawaz Dec 16 '13 at 16:07
  • @H2CO3: They're only reserved for use in the global namespace. They're merely ugly in other scopes, and perfectly valid for use by those who think baroque squiggles enhance their code. – Mike Seymour Dec 16 '13 at 16:07
  • 1
    I prefer a trailing `_` just for that reason so I don't have to remember those rules. – greatwolf Dec 16 '13 at 16:07
  • @greatwolf: This rule is so simple that if I know it once, I know it for all eternity! – Nawaz Dec 16 '13 at 16:10
  • Although I used to use trailing underscores, I'm not a fan of the leading underscore at all. It's too close to invalid according to the Standard for my comfort. Otherwise agreed with answer. – John Dibling Dec 16 '13 at 16:10
  • @MikeSeymour: If that is true, then what names are reserved for internal implementation macros (which don't respect scopes at all)? – John Bartholomew Dec 16 '13 at 16:12
  • 2
    @JohnBartholomew: Names containing two consecutive underscores, or beginning with an underscore and an upper-case letter. – Mike Seymour Dec 16 '13 at 16:13
  • @MikeSeymour: Never mind; I see you were referring specifically to single-leading-underscore names. – John Bartholomew Dec 16 '13 at 16:13
  • I've successfully argued to use `m_` as a prefix for member variables at several places. It distinguishes your variables and has no potential to run into issues with reserved names. – Zac Howland Dec 16 '13 at 16:13
  • @ZacHowland: When `_name` is fine, I find no reason to type one more character, to make it `m_name`. In template programming, it often makes some lines longer than necessary. I used to follow this when I didn't use to write template programming. – Nawaz Dec 16 '13 at 16:16
  • @Nawaz That gets into a preference ... mine is largely a holdover from when I was brainwashed into using Hungarian Notation years ago. While I've dropped the type prefixes, I still hold onto the member, static, member static, and global prefixes (`m_`, `s_`, `ms_`, and `g_`, respectively). – Zac Howland Dec 16 '13 at 16:19
  • The Google C++ Styleguide specifies a trailing underscore for member variables (since m_var would be Hungarian and _var would be using a reserved (for implementation) variable name) http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Variable_Names#Variable_Names – Duncan Smith Dec 16 '13 at 16:27
  • @DuncanSmith: `_var` is NOT reserved for implementation; `__var` and `_Var` are. – Nawaz Dec 16 '13 at 16:34
  • Sure, I just meant var to be a place-holder for the actual variable name. I didn't mean to imply any case conventions - sorry for any confusion. – Duncan Smith Dec 16 '13 at 16:36
  • @DuncanSmith `m_` is **not** Hungarian. It is a small part of Hungarian Notation, but it goes **much** deeper than that. – Zac Howland Dec 16 '13 at 16:38
  • @DuncanSmith: Still `_anyname` is NOT reserved. – Nawaz Dec 16 '13 at 16:45
  • @Nawaz, if the chosen var name was Anyname, then _Anyname would be reserved. (From the gnu c lib manual: and all identifiers regardless of use that begin with either two underscores or an underscore followed by a capital letter are reserved names) – Duncan Smith Dec 16 '13 at 16:49
  • @DuncanSmith: Yes. that is true. That is exactly what my answer says, no? – Nawaz Dec 16 '13 at 16:57
  • @Nawaz: Yes, you're answer is correct. As was mine when I said that I didn't imply any case conventions - so 'var' could be substituted for 'Var' - it sounds like we're in agreement. – Duncan Smith Dec 16 '13 at 17:04
3

This is permitted by the C++ standard, but consider the case where, after initializing members, you want to do more work in the function. For example, using 3 to stand in for some more meaningful calculation:

class Foo
{
public:
    int bar;
    Foo(int bar) : bar(bar) { bar = 3; }
};

The assignment in the function changes the value of the parameter bar, not the value of the member bar. This cannot happen in your example, because you declared the parameter with const. So, if you are sure to always declare parameters with const, you are protected from this. But consider a more complicated scenario:

class Foo
{
public:
    int bar;
    int baz;
    void AuxiliaryFunction() { bar = 3; }
    Foo(const int &bar) : bar(bar)
    {
        AuxiliaryFunction();
        baz = bar;
    }
};

In this example, the member bar is given some value through another function called in the constructor. Then the assignment baz = bar; may be intended to copy the member bar, but it actually copies the parameter bar.

So, while this is legal C++, it should be used judiciously.

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
2

Yes. It does compile. For the compiler, there is no ambiguity of which value is which.

#include <iostream>

using namespace std;

template <typename T>
struct Struct {
    Struct(const T & value) : value(value) {}
    T value;
};

int main() {

    Struct<int> T(1);
    // your code goes here
    return 0;
}

http://ideone.com/gPyBK6

But at stated multiple times, this is not easy for the programmer to decipher. It does work because for the compiler the parameter mask the member function, so the second value in value(value) is the parameter, but as only member and ancestor class can be on the left part of value(value), it does refer to the member here.

Tricky enough to complicate debugging and maintenance.

Johan
  • 3,728
  • 16
  • 25
  • It does compile, but there is ambiguity (not by the compiler - but by future readers of your code). – Zac Howland Dec 16 '13 at 16:02
  • @jrok I was in the middle of editing ... anytime you see `value(value)` in an initializer list, a pony dies. – Zac Howland Dec 16 '13 at 16:04
  • Arguably, it's bad practice to call the ivar and the initializer list argument the same, but - unfortunately - it's valid. –  Dec 16 '13 at 16:04
  • @ZacHowland That's true, but this fact doesn't render this answer technically incorrect. –  Dec 16 '13 at 16:04
  • @ZacHowland I speak from the compiler point of view, will edit and make an more apropriate statement. – Johan Dec 16 '13 at 16:05
  • @H2CO3 I didn't downvote, but I imagine it is due to the lack of clarity in the code. – Zac Howland Dec 16 '13 at 16:06
  • @ZacHowland There's no "lack of clarity" in the code (whatever you mean by that). –  Dec 16 '13 at 16:06
  • @H2CO3 Every time I've seen code like that in a code review, the writer has left with scars on their back from the flogging they took. You write code so that a future reader can easily understand it ... making to variables have the same name is syntactically correct, but would require the reader to know about the minutia of that rule. There is no reason why a member variable and a function parameter to set it should share the same name. Hence, the lack of clarity. – Zac Howland Dec 16 '13 at 16:10
  • @ZacHowland Yeah. Did I say it's good? Nope. Did I say it's bad? Yes, I did. Just read my second comment. That's not "ambiguity" in the technical sense of the word. No, don't do this. Yes, it does compile. Unfortunately. No, that doesn't mean the author of this answer should be punished for OP writing crappy code. –  Dec 16 '13 at 16:12
  • @H2CO3 Don't shoot the messenger ... I don't disagree with you :) – Zac Howland Dec 16 '13 at 16:21
  • 1
    @ZacHowland The messenger is slowly recovering from his flesh wounds :p – Johan Dec 16 '13 at 16:25
2

It is valid, but ill-advised in some circles, including my circle.

It is valid in the sense that the member variable will be properly set by the parameter as you desire. Following the execution of the initializer list, the member will be hidden. Any reference to value will access the parameter. This is likely a bad thing.

It is ill-advised for two reasons. First and foremost, maintainability & confusion. It is unusual to see a parameter and member variable have the same name. Because of this, most programmers will have to stop and think about what it means. After all, you did. Remember that code is written first for programmers, second for compilers. Code that is easy to understand is much better than code that is hard to understand. In a code review I would reject this code on these grounds.

Second, the member hiding will likely be a problem in most scenarios.

I'd suggest coming up with a sane naming scheme and sticking with it. "Sane" means a parameter can never have the same name as a member variable. For example in my naming scheme, member variables are always prepended with m -- parameters never prepended. So in this scheme your code would become:

struct Struct {
  Struct(const T& value) : mValue(value) {}
  T mValue;
};

Using this scheme, nobody is confused about what's happening here and nobody has to ask StackOverflow "is this legit?"

John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • IIRC, isn't the member hidden by the parameter, not the other way around? – Zac Howland Dec 16 '13 at 16:17
  • Are you sure there is a name hiding issue? I think section `12.6.2 paragraph 2` would prevent that or do I misunderstand? I think the look rule for the `expression-list` is implicitly outside the class definition. I am referring to this [draft](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3485.pdf). – Shafik Yaghmour Dec 16 '13 at 16:23
1

It is valid. However a little warning: Changing the argument name and the code is undefined behavior.

template <typename T>
struct Struct {
    Struct(const T & argument) : value(value) {}
    T value;
};
0

It's valid but, as evidenced by the need to even ask this, potentially jarring and confusing.

The perennial problem is coming up with two distinct names for what is kinda the same thing.

I like to do this (but some people hate it):

struct Struct {
  Struct(const T& newValue) : value(newValue) {}
  T value;
};