c++ declaring a class's const member object

Question

I'm referring to this stackoverflow post:

When I build the following code, I get: "duplicate symbol for architecture x86_64". I've been googling for the past couple of hours, but have not found anything to help. Any suggestions?

#ifndef __lexer__lexer__
#define __lexer__lexer__

#include <stdio.h>
#include <string>

namespace Tag{
  const int AND = -1,
  OR = -2;
}

class Token{
public:
  std::string Name;
  std::string Type;
  std::string Value;
  int Tag;
  Token(std::string name, std::string type, std::string value, int tag){
    Name = name;
    Type = type;
    Value = value;
    Tag = tag;
  }
};

class Word : public Token{
public:
  Word(std::string name, std::string type, std::string value, int tag) : Token(name, type, value, tag){}
  static const Word AND;
};

const Word Word::AND = *new Word("and", "logical", "&&", Tag::AND);

#endif /* defined(__lexer__lexer__) */

The code:

const Word Word::AND = *new Word("and", "logical", "&&", Tag::AND);

Is what is giving me the problems.

Have a look at `Tag = tag;` and `int Tag;`, it's probably not a very good idea to give the member variable the same name as the namespace. That cries out for ambiguity! — πάντα ῥεῖ, Nov 19 '14 at 18:55
Unrelated: Names beginning with a double underscore are reserved by the implementation. — David G, Nov 19 '14 at 18:56

score 4 · Accepted Answer · answered Nov 19 '14 at 18:57

4

The short answer is you don't want to do the definition (as opposed to the declaration) in a .h file. Your error comes when the .h file is included in more than one other file.

A slightly longer answer is that your *new idiom is unnecessary and probably wastes a small amount of storage.

const Word Word::AND("and", "logical", "&&", Tag::AND);

will invoke the same constructor.

And an even longer answer is that Tag should be an enum, and you do not want to pass std::string by value. Maybe you are coming from another language: you need to learn about pass by reference and const reference.

answered Nov 19 '14 at 18:57

Andrew Lazarus

18,205
3
35
53

1

The `*new` is not just unnecessary, it stinks. – Jonathan Wakely Nov 19 '14 at 19:03
I am coming from another language, C#, how would you recommend passing by reference in this instance? – Chase W. Nov 19 '14 at 19:10
1

"you do not want to pass std::string by value" In the context of constructor and when `std::move`ed correctly in the member initializer list, you actually do. It's the simplest thing to do to effectively handle lvalue (inner copie gets elided by the compiler)/rvalue arguments + you cannot take advantage of preallocated storage inside the `std::string`s anyway (as you could when writing e.g. a setter). – Jiří Pospíšil Nov 19 '14 at 19:10
@JiříPospíšil Doesn't that require `std::string&&` in the header? – Andrew Lazarus Nov 19 '14 at 19:20
@ChaseW. Basically, you need to get a guide for differences between C# and C[++]. As I understand C#, like Java, does all its own memory management, and all calls are by reference. C++ has more flexible, and also more dangerous, memory management _by the programmer_. No way to summarize that in a comment. But that is also why `new` for a const object is not only unnecessary but wasteful. – Andrew Lazarus Nov 19 '14 at 19:22
1

@AndrewLazarus No. That would bind to rvalue references only. What you're probably thinking of are universal (or forwarding) references. That's another valid way to go but different from this one. Forwarding references require you to take a non const rvalue reference of a template parameter (because the concept depends on reference collapsing) and then `std::forward`ding of the argument to the final location. – Jiří Pospíšil Nov 19 '14 at 19:28
The *new doesn't just stink. It leaks memory. The new allocates memory, the *dereferences it and the = assigns (copies) the value to Word::AND. The allocated object is lost and will and can never be freed. – Goswin von Brederlow Nov 20 '14 at 10:12

score 2 · Answer 2 · answered Nov 19 '14 at 18:55

2

The initialization code belongs in a .cc file, not in the header. In the header every compilation unit that includes it would create a new one and then the linker rightfully complains about duplicates.

answered Nov 19 '14 at 18:55

Goswin von Brederlow

11,875
2
24
42

c++ declaring a class's const member object

2 Answers2