views:

171

answers:

5

I have several members in my class which are const and can therefore only be initialised via the initialiser list like so:

class MyItemT
{
public:
    MyItemT(const MyPacketT& aMyPacket, const MyInfoT& aMyInfo)
        : mMyPacket(aMyPacket),
          mMyInfo(aMyInfo)
    {
    }

private:
    const MyPacketT mMyPacket;
    const MyInfoT mMyInfo;
};

My class can be used in some of our internally defined container classes (e.g. vectors), and these containers require that operator= is defined in the class.

Of course, my operator= needs to do something like this:

MyItemT&
MyItemT::operator=(const MyItemT& other)
{
    mMyPacket = other.mPacket;
    mMyInfo = other.mMyInfo;
    return *this;
}

which of course doesn't work because mMyPacket and mMyInfo are const members.

Other than making these members non-const (which I don't want to do), any ideas about how I could fix this?

+2  A: 

It's a dirty hack, but you can destroy and reconstruct yourself:

MyItemT&
MyItemT::operator=(const MyItemT& other)
{
    if ( this == &other ) return *this; // "suggested" by Herb Sutter ;v)

    this->MyItemT::~MyItemT();
    try {
        new( this ) MyItemT( other );
    } catch ( ... ) {
        new( this ) MyItemT(); // nothrow
        throw;
    }
    return *this;
}

Edit: lest I destroy my credibility, I don't actually do this myself, I would remove the const. However, I've been debating changing the practice, because const simply is useful and better to use wherever possible.

Sometimes there is a distinction between the resource and the value represented by an object. A member may be const through changes to value as long as the resource is the same, and it would be nice to get compile-time safety on that.

Edit 2: @Charles Bailey has provided this wonderful (and highly critical) link: http://gotw.ca/gotw/023.htm.

  • Semantics are tricky in any derived class operator=.
  • It may be inefficient because it doesn't invoke assignment operators that have been defined.
  • It's incompatible with wonky operator& overloads (whatever)
  • etc.

Edit 3: Thinking through the "which resource" vs "what value" distinction, it seems clear that operator= should always change the value and not the resource. The resource identifier may then be const. In the example, all the members are const. If the "info" is what's stored inside the "packet," then maybe the packet should be const and the info not.

So the problem isn't so much figuring out the semantics of assignment as lack of an obvious value in this example, if the "info" is actually metadata. If whatever class owns a MyItemT wants to switch it from one packet to another, it needs to either give up and use an auto_ptr<MyItemT> instead, or resort to a similar hack as above (the identity test being unnecessary but the catch remaining) implemented from outside. But operator= shouldn't change resource binding except as an extra-special feature which absolutely won't interfere with anything else.

Note that this convention plays well with Sutter's advice to implement copy construction in terms of assignment.

 MyItemT::MyItemT( MyItemT const &in )
     : mMyPacket( in.mMyPacket ) // initialize resource, const member
     { *this = in; } // assign value, non-const, via sole assignment method
Potatoswatter
@Potatoswatter - Thanks for the suggestion, but I think this is probably dirtier than making the members non-const to start with.
LeopardSkinPillBoxHat
@Leopard: understood, but on the other hand there is nothing really unsafe going on here. In a way, you get an additional guarantee that no stale state is being used.
Potatoswatter
If the copy constructor throws, you might be doomed, because as far as the language goes, `*this` will be inevitable destroyed again.
UncleBens
@Uncle: fixed, as far as its possible to fix that.
Potatoswatter
It's now added a requirement for a no-throw default constructor (which may not be possible depending in the class' members and bases). It's unsafe for self-assignment. It's really bad if this class is a base class for anything. I'd agree. It could definitely be seen as a dirty hack. http://www.gotw.ca/gotw/023.htm
Charles Bailey
David Rodríguez - dribeas
@David: I suppose. What it gets down to is the meaning and utility of `const`. Sometimes there is a distinction between the resource and the value represented by an object. A member may be `const` through changes to value as long as the resource is the same, and it would be nice to get compile-time safety on that. Like I said, I'm kind of playing devil's advocate here.
Potatoswatter
+7  A: 

You're kind of violating the definition of const if you have an assignment operator that can change them after construction has finished. If you really need to, I think Potatoswatter's placement new method is probably best, but if you have an assignment operator your variables aren't really const, since someone could just make a new instance and use it to change their values

Michael Mrozek
@Michael - True, and a valid point.
LeopardSkinPillBoxHat
On the other hand, nothing actually prevents the user from performing such evil manually. (Or just using `const_cast`!) Certainly, document that `operator=` invalidates references to members.
Potatoswatter
Well, between `const_cast` and the `mutable` specifier you can pretty thoroughly destroy the concept of const variables :). There's little C/C++ will actually prevent you from doing, if you force it to
Michael Mrozek
@Michael - I think there's a difference between someone who intentionally shoots themselves in the foot and one who accidentally does, and I think `const` is there to protect the latter. At least the usage of `const_cast` et al. should appear as a flashing red light (or a really bad smell) to most people who use it or find it in code.
LeopardSkinPillBoxHat
The classical phrase is that C++ protects against Murphy, not Machiavelli.
MSalters
There are reasons why you might want to have an assignable class even if you don't want to 'normally' allow a part of that class to be modified. STL containers require that the objects stored in them be 'Assignable' types. Until C++0x's 'move' semantics are widely available, objects that contain `const` members yet are assignable will have some legitimacy.
Michael Burr
+3  A: 

Rather than storing objects in your containers directly, you might be able to store pointers (or smart pointers). That way, you don't have to mutate any of the members of your class -- you get back exactly the same object as you passed in, const and all.

Of course, doing this will probably change the memory management of your application somewhat, which may well be a good enough reason not to want to.

Andrew Aylett
@Andrew - Interesting suggestion, but probably not practical in my case. My constructor arguments are stack allocated objects so I need to take a copy of them into my class due to lifetime issues (as my class will outlive the stack objects).
LeopardSkinPillBoxHat
@LeopardSkinPillBoxHat: even if the class maintains pointers, that doesn't mean you'll have lifetime issues - the class would still copy that data into it's own allocations, it's just that they'd be `new`ed off the heap with pointers (or smart pointers) to those dynamically allocated bits rather than having the copies directly in the object.
Michael Burr
+1  A: 

You might consider making the MyPacketT and MyInfoT members be pointers to const (or smart pointers to const). This way the data itself is still marked const and immutable, but you can cleanly 'swap' to another set of const data in an assignment if that makes sense. In fact, you can use the swap idiom to perform the assignment in an exception safe manner.

So you get the benefit of const to help you prevent accidentally allowing changes that you want the design to prevent, but you still allow the object as a whole to be assigned from another object. For example, this will let you use objects of this class in STL containers.

You might look at this as a special application of the 'pimpl' idiom.

Something along the lines of:

#include <algorithm>    // for std::swap

#include "boost/scoped_ptr.hpp"

using namespace boost;

class MyPacketT {};
class MyInfoT {};


class MyItemT
{
public:
    MyItemT(const MyPacketT& aMyPacket, const MyInfoT& aMyInfo)
        : pMyPacket(new MyPacketT( aMyPacket)),
          pMyInfo(new MyInfoT( aMyInfo))
    {
    }

    MyItemT( MyItemT const& other)
        : pMyPacket(new MyPacketT( *(other.pMyPacket))),
          pMyInfo(new MyInfoT( *(other.pMyInfo)))
    {   

    }

    void swap( MyItemT& other) 
    {
        pMyPacket.swap( other.pMyPacket);
        pMyInfo.swap( other.pMyInfo);        
    }


    MyItemT const& operator=( MyItemT const& rhs)
    {
        MyItemT tmp( rhs);

        swap( tmp);

        return *this;
    }

private:
    scoped_ptr<MyPacketT const> pMyPacket;
    scoped_ptr<MyInfoT const> pMyInfo;
};

Finally, I changed my example to use scoped_ptr<> instead of shared_ptr<> because I thought it was a more general representation of what the OP intended. However, if the 'reassignable' const members can be shared (and that's probably true, given my understanding of why the OP wants them const), then it might be an optimization to use shared_ptr<>'s and let the copy and assignment operations of the shared_ptr<> class take care of things for those objects - if you have no other members that require special copy or assign sematics, then your class just got a lot simpler, and you might even save a significant bit of memory usage by being able to share copies of the MyPacketT and MyInfoT objects.

Michael Burr
+1  A: 

I think you could get away with a special const proxy.

template <class T>
class Const
{
public:
  // Optimal way of returning, by value for built-in and by const& for user types
  typedef boost::call_traits<T>::const_reference const_reference;
  typedef boost::call_traits<T>::param_type param_type;

  Const(): mData() {}
  Const(param_type data): mData(data) {}
  Const(const Const& rhs): mData(rhs.mData) {}

  operator const_reference() const { return mData; }

  void reset(param_type data) { mData = data; } // explicit

private:
  Const& operator=(const Const&); // deactivated
  T mData;
};

Now, instead of const MyPacketT you would have Const<MyPacketT>. Not that the interface only provides one way to change it: through an explicit call to reset.

I think any use of mMyPacket.reset can easily be search for. As @MSalters said it protects against Murphy, not Machiavelli :)

Matthieu M.