4

I am trying:

class MyClass {
public:
    MyClass();

    int myID;
    static int currentID;
};

MyClass::MyClass() {
    myID = currentID;
    ++currentID;
}

I'm trying to assign a unique ID to all the instances of this class.

Edit:

It doesn't work for me. I get two of these in xcode:

Undefined symbols: "GameObject::currentID", referenced from: __ZN10GameObject9currentIDE$non_lazy_ptr in GameObject.o (maybe you meant: __ZN10GameObject9currentIDE$non_lazy_ptr) ld: symbol(s) not found collect2: ld returned 1 exit status

Ryan
  • 5,883
  • 13
  • 56
  • 93
  • 1
    Ideally, you would look at the singleton that uses boost::once to see how race-free initialization of statics works. You should also use tbb::atomic / std::atomic to prevent race conditions. Those two modifications complete, your code is now thread-safe. – moshbear Dec 03 '11 at 12:15
  • Run-time-wise, yes. Compile-time-wise, you need to (re-)define `int MyClass::currentID` outside of class scope. – moshbear Dec 03 '11 at 12:20
  • @moshbear: technically he needs to *define* what he has provided is a *declaration* (i.e. the `re-` in *(re-)define* should not be there) – David Rodríguez - dribeas Dec 03 '11 at 12:54
  • Be careful if you plan to use MyClass in STL containers. The code may not behave the same as you expected. – Lei Mou Dec 03 '11 at 14:55
  • @DavidRodríguez-dribeas Hence the parentheses to indicate uncertainty. I tend to forget the distinction between declaration and definition. – moshbear Dec 03 '11 at 23:45

7 Answers7

5

It works for me:

#include <iostream>

class My
{
    public:
    My() : id(++currentId) {}

    int id;
    static int currentId;
};

int My::currentId = 0;

std::ostream & operator<<(std::ostream & os, My & m)
{
    return os << m.id;
}

int main()
{
    My a;
    My b;
    My c;

    std::cout << a << std::endl;
    std::cout << b << std::endl;
    std::cout << c << std::endl;
}

Output:

> ./x
1
2
3
stefanB
  • 77,323
  • 27
  • 116
  • 141
  • Didn't have the initializer like vrbilgi mentions. You were the first to write it. Why does there need to be an int in front, why not just: My::currentId = 0; – Ryan Dec 03 '11 at 12:28
  • @Ryan: That is not *initialization* (or rather not *only* initialization), but rather the actual *definition* of the static member: `int My::currentId/* = 0*/;` *defines* the variable (reserves the space for the variable), removing the comment it also *initializes*, but the important bit is that this is the *definition* – David Rodríguez - dribeas Dec 03 '11 at 12:57
  • @stefanB: The last comment makes no sense: the variable is not `const` – David Rodríguez - dribeas Dec 03 '11 at 12:57
  • it is the syntax of initialising static class variables – stefanB Dec 03 '11 at 13:09
3

Besides the others' cases, it works in the following cases:

MyClass a[10];
MyClass* b = new MyClass[10];

Be careful if you use STL containers to hold MyClass objects. It may not behave the same as you expected, and the problem is hard to find. See the following example:

int MyClass::currentID = 0;
...
std::vector<MyClass> c(10);
MyClass a;

In this case, The result is the following:

c[0].myID = 0;
c[1].myID = 0;
....
c[9].myID = 0
=============
   a.myID = 1. 

The default constructor is executed only ONE time. The reason is that the constructor of std::vector will use the same value to initial all the element of the vector. In my system, the constructor of std::vector is the following:

  explicit
  vector(size_type __n, const value_type& __value = value_type(),
     const allocator_type& __a = allocator_type())
  : _Base(__n, __a)
  { _M_fill_initialize(__n, __value); }   

_M_fill_initialize will initialize the memory allocated using __value (which comes from default constructor value_type()) __n times.

The code above will eventually call uninitialized_fill_n, which does the following:

for (; __n > 0; --__n, ++__cur)
    std::_Construct(&*__cur, __x);

Here is std::_Contruct:

template<typename _T1, typename _T2>
inline void
_Construct(_T1* __p, const _T2& __value)
{
  // _GLIBCXX_RESOLVE_LIB_DEFECTS
  // 402. wrong new expression in [some_]allocator::construct
  ::new(static_cast<void*>(__p)) _T1(__value);
}

Here you can see it finally calls global operator new to initialize each element in the vector using the same value.

The conclusion is, using static member to initial data member will work in most of the cases, but it may fail if you plan to use it in STL containers, and the problem is hard to find. I only tried std::vector, it is quite possible that the problem exists when using other kind of stl containers with MyClass objects.

Lei Mou
  • 2,562
  • 1
  • 21
  • 29
  • 1
    The constructor for `vector` in c++98 is in fact as you said, and the default constructor is only called to form the default argument. But in C++0x, the specification split this constructor in two: `explicit vector(size_type n);` and `vector(size_type n, const T& value, const Allocator& = Allocator());`. for the first one, the specification says the vector should `Constructs a vector with n value-initialized elements.`, which requires the default constructors to be called `n` times. Anyway, the approach of the OP should be used with caution in this situation. – fefe Dec 03 '11 at 15:38
  • I am using MyClass with stl containers, but I am only using stl containers to hold pointers to MyClass. For example: myVector.push_back(new MyClass()); Is it okay to use like that? – Ryan Dec 03 '11 at 19:31
  • 1
    @Ryan It is OK then, just make sure that each time the default constructor is called and you know what exactly happening under the hood. – Lei Mou Dec 04 '11 at 02:56
1

You need to make sure that you define and initialize static member of the class outside the class. This works for me:

#include <iostream>
class MyClass {
private:
    int myID;
    static int currentID;
public:
   //default constructor
    MyClass()
        : myID(currentID) 
        {
            ++currentID;
        }

    void printId() { std::cout << "Object Id :" << myID << std::endl; } /*Prints the unique ID assigned to each object.*/
};

// define the static class member here:
int MyClass::currentID = 1;

int main()
{
    MyClass m1, m2, m3;
    m1.printId();
    m2.printId();
    m3.printId();
}

Output:

Object Id :1

Object Id :2

Object Id :3

Shakti S
  • 11
  • 2
1
int MyClass::currentID=0;  

If not done you need to do this in CPP file

vrbilgi
  • 5,703
  • 12
  • 44
  • 60
1

The reason you're getting the error is not because you're accessing the static from the constructor, which is legal btw, but because you're not defining it.

class My
{
    public:
    My() : id(++currentId) {}

    int id;
    static int currentId;
};

//place the definition in the implementation file
int My::currentId = 0;
stefanB
  • 77,323
  • 27
  • 116
  • 141
Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • @stefanB: If you are suggesting that he copied, I would not think so, there are not that many things that can be done so he can have got to the same code *without* copying, and besides, contrary to your version, he actually *explains* in the code with the comment what the line does: *defininion of the variable*. +1 for the answer that actually *mentions* that the missing part is the definition – David Rodríguez - dribeas Dec 03 '11 at 13:00
0

yes that is fine, provided you have no static instances of MyClass.

justin
  • 104,054
  • 14
  • 179
  • 226
  • @Ryan that's a different issue. you will need to *define* the static variable in the cpp file -- as vrbilgi, StefanB, and Luchian have now demonstrated for you. – justin Dec 03 '11 at 12:23
0

Yes, it's ok. But the code as you wrote it is not thread-safe: 2 classes can be instantiated simultaneously in different threads. The 2nd instance could grab the same value of currentID before the first instance had time to increment the static variable.

This may or may not be an issue for you.

If you need thread-safety, this question should help.

Community
  • 1
  • 1
Serge Wautier
  • 21,494
  • 13
  • 69
  • 110