ali_bahoo
ali_bahoo

Reputation: 4863

C++ Constness problem

I have a problem with whether making class members constant or not. Let me give an example.

#include <iostream>
class ValueClass
{
    int ival;
public:
    void set(int i) {ival = i;}
    int get() {return ival;}
};

class XXX;

class ABC
{
    ValueClass vc;  
public:
    int getval() const {return vc.get() ;}  
friend class XXX;
};

class XXX
{
    std::vector<ABC> abclist;
    void Invalidate()
    {
    // Iterates through abclist and modifies ValueClass members.
    // e.g. abclist[i].vc.set(i);
    }
};

class QWE
{
    const ABC & abc; 
public:
    QWE(const ABC & abc_): abc(abc_) { }
    const ABC & getABC() { return abc; } 
};

int main()
{
    ABC abc;
    QWE qwe(abc);

    std::cout << qwe.getABC().getval()  << "\n"; // Compiler error
}

My ABC class holds an instance of ValueClass which is responsible for having getter and setter for an int value. Also QWE class has an ABC member and I need to obtain this member. I am told that returning the abc as non-const is a very bad practice. But here comes the problem, I can not use non-const vc in a const int getval() function and I simply can not make it const because in another thread XXX::Invalidate() is called. This function changes the data in ValueClass with respect to some incoming data.
Obviously there is something wrong with my design, I can not blame the C++ language. How can I solve this problem?

Upvotes: 1

Views: 180

Answers (3)

James McNellis
James McNellis

Reputation: 354969

As others have noted, the accessors should be const-qualified. As a general rule, if something can be const-qualified, it should be const-qualified (there are a few exceptions, of course, e.g. the return type of a function that returns by value should not be const-qualified, though it usually doesn't matter there).

However, there is another issue with the code as written: it is very easy to use incorrectly and accidentally lose track of your object lifetimes. Consider if you had instead declared qwe in main() as:

QWE qwe(ABC());

The ABC() constructs a temporary ABC object and passes it to the QWE constrcutor. Since the constructor parameter abc_ is a const ABC&, it will bind to that temporary object. You then bind the member reference abc to the temporary object via the initializer abc(abc_). The constructor then returns.

After the constructor returns, the temporary ABC object is destroyed and qwe.abc is a dangling reference: it no longer refers to an object.

If a class is going to hang onto a reference that you pass to it, you should prefer to use a pointer for the parameter type: this way, it is clearer that you have to concern yourself with potential lifetime issues. In addition, reference-type class members are usually more mess than they are worth, since they make the class non-assignable (since references themselves are not assignable).

Upvotes: 1

Erik
Erik

Reputation: 91260

class ValueClass
{
    ...
    int get() const {return ival;}
};

class QWE
{
    ...
    const ABC & getABC() const { return abc; } 
};

Your code must be const-correct throughout or you'll run into this problem all the time.

Upvotes: 0

Lou Franco
Lou Franco

Reputation: 89142

Make get() const in ValueClass

  class ValueClass
  {
      int ival;
    public:
      void set(int i) {ival = i;}
      int get() const {return ival;} // make get const
  };

Probably should do the same for getABC

  const ABC & getABC() const { return abc; } 

This means that this method does not change the object it is called on.

Upvotes: 3

Related Questions