Does it make sense to implement the copy-assignment operator in a class with all const data members?
Asked Answered
W

6

2

Imagine I have a class used to represent some trivial numerical data, like a vector. I want this class to be immutable in terms of its members, but still support correct copy-assignment behaviour.

This is what the shell of this class might look like:

class Vector {
  public:
  Vector(float _x, float _y);

  private:
  const float x;
  const float y;
};

I would like to be able to assign a new value to a vector member within (for example) a class. An on-screen entity with a position, perhaps, represented using a non-const member of type Vector.

This 'entity' must be mutable itself (perhaps supporting movement), e.g. the internal Vector member used to store its position must change. Rather than modifying this member directly, I would rather replace it with an entirely new Vector instance, initialised with the modified values.

In a language such as C#, I would simply compute the new values for X and Y based on the current Vector instance, and assign a new Vector object initialised with these values to the member, discarding the previous value and letting the GC do the rest.

// Inside some class (C#/Java type)...
Vector position;
...
// Example function:
void Move(float _dX, float _dY) {
  this.position = new Vector(_dX + position.X, _dY + position.Y);
}

This is where my limited ability with C++ comes into play, because I am unsure of how to implement operator= in such a way that instances of this immutable type contained in dynamic memory do not simply 'disappear' and cause a leak.

So, I suppose, what I am asking is this:

Vector& operator=(const Vector& _rhs);
...
// In implementation file...
Vector& Vector::operator=(const Vector& _rhs) {
  // WHAT ON EARTH GOES IN HERE?!?!?
}

The Vector case is something I invented to get my point across, I can think of many sorts of types that should be immutable at the member level.

Am I wasting my time attempting to model immutable types?

Is there a better solution?

Whacking answered 7/9, 2010 at 12:57 Comment(3)
As long as no methods can modify the internal members then the object is immutable. So just mark all member methods as const. This the actual members to be normal and allows normal copy assignment.Perionychium
Why not get a C++ book so you can start thinking in C++ when you program in C++. Thinking in C# isn't going to help you at all, it's just going to hurt you. I'd say ditch the const-members, they're generally code smell.Quieten
What I have done wrong is made the assumption that the = operator in the C# example is the counterpart to the copy-assignment operator in C++, which it isn't. Thanks, everyone, for the responses.Whacking
P
3

When you type

Vector position;

in a class definition, you define an instance of a Vector class that will stay here forever. If that one is immutable, you're screwed.

In C#/Java, there is no way to do this. Instead you define references.

If you want to do the same thing in C++, you need pointers or auto_pointers

Vector* position;

That way, you can change the instance to which position is pointing.

You shouldn't define the assignment operator of a immutable class. If you really need to, maybe you should make Vector mutable, and use const Vector when you need an immutable Vector. You will just need to add const qualifiers to method that are allowed with a const Vector.

eg:

class Vector {
    private:
    float x;
    float y;
    public:
    // allowed for const Vector
    // because of the const before the '{'
    float norm() const {
        return hypot(x,y);
    }

    Vector& operator=(const Vector& o) {
        x=o.x;
        y=o.y;
        return *this;
    }
};
Phasis answered 7/9, 2010 at 12:57 Comment(3)
I see your point, it did not occur to me at all that inside the C++ version of the client class the Vector object would be a reference/pointer, in which case I can simply delete it and create a new one. Still getting used to the manual handling of pointers/refs.Whacking
@p34ce: Don't get used to it. Modern C++ doesn't manually free any resource, look into Scope-Bound Resource Management (or Resource-Acquisition is Initialization). I strongly recommend you get a book on the subject.Quieten
From what I can tell, the problem with what I am trying to do is that the assignment operator should return a reference to self, to preserve left-associativity. When the members are declared as const, returning ref-to-self wouldn't return an object with the new values, returning a new object would produce a garbage reference, and returning the right-hand value wouldn't do anything. I think bitwise const methods coupled with a default copy-assignment operator is the correct way to do this. Not 100% clear on RAII, but I'm getting to it, smart pointers, exception safety etc.Whacking
O
2

What you're trying to do doesn't make sense for use with the operator=. Reason being is you need an instance of your Vector class in order to use operator=, but you've stated you do not want to allow changes to the internals.

Normally, your operator= would like something like:

Vector::operator=(const Vector &rhs)
{
   x = rhs.x;
   y = rhs.y;
}

* Note the compiler would automatically generate this for you and it would be safe as you have no pointers,etc.

This operator= overload won't work for your scenario, b/c the data members x and y will not reassign b/c you have them as const. This is where you would use a copy-constructor with an initialization list. If you really wanted to have operator=, you could use const_cast inside the overload, but most would consider this poor form.

Vector::Vector(const Vector &copy)
   :x(copy.x), y(copy.y)
{

}

* Note the compiler would also automatically generate this type of copy constructor for you.

Then your code would create the object with

Vector newObj(oldObj);

Having stated all this to answer your question, why not just let the members be non-const and declare the Vector const?

const Vector myVector(5,10);
Ostosis answered 7/9, 2010 at 13:2 Comment(10)
Thanks, that makes sense, in fact I already have a copy-constructor defined - I was thinking in terms of the Rule of Three.Whacking
Nice. Wish I'd thought of that last week.Freemon
@p34ce: Rule of three does not apply as you are not managing any resources in your object.Perionychium
@p34ce: As all rules of thumb, sometimes the Rule of Three doesn't apply. Nevertheless, I don't see why this would be the case in this example. That copy ctor isn't any different from what the compiler creates for you and from what you posted a dtor isn't needed. The case that none of the Three are needed is covered by the Rule of Three.Peisch
@p34ce - Rule of three is more directed for situations where you are using pointers and you're creating or locking resources. In these cases you need a copy constructor b/c of the need for a deep copy, and if you need this behavior in the CC, then you'll need it in the assignment op. The destructor will then need to be defined in order to free those resources.Ostosis
Thanks for the tips, I am working through a few books (Effective C++ 3ed and Gotchas), only been at it for a couple of days, still getting the hang of it.Whacking
sbi is correct in stating that this copy constructor would be the same as what would be auto generated by a compiler. I was showing the code for the CC to be clear as to what would be used to create his new instance and how it would be called. I will update the answer stating that defining this would be unnecessary.Ostosis
Rule of Three does apply here since the compiler-generated assignment operator cannot be used. In this particular case the right thing to do is declare the assignment operator as private and then either not define it or define it to assert if called.Radium
Thanks RC. What I'm doing here I think is trying to apply a member-immutability concept from other languages, which doesn't transplant to C++ completely, because the logic isn't hidden by a GC. I'm accustomed to there being no syntactic difference between refs/values, á la Java/etc.Whacking
@Ben: I see no benefit in declaring a private assignment operator. Just leaving it up to the compiler has exactly the same result: it cannot be used.Peisch
Z
2

You have a logical inconsistency in your requirements. On the one hand, all objects of class Vector must be immutable, but on the other hand you want to be able to change them. These two requirements are in direct conflict with each other.

A better design might be to make the classes such that the only way to change the value is to assign a new object of the class to it:

class Vector {
public:
  Vector(float _x, float _y);
  /* Vector(const Vector&) = default; */
  /* Vector& operator=(const Vector&) = default; */

  float x() const;
  float y() const;
  /* No non-const accessors!! */
private:
  float x;
  float y;
};

Once initialised, instances of this class can't be modified, except by completely overwriting them with a new instance.

Zoomorphism answered 7/9, 2010 at 13:45 Comment(0)
I
1

If you're concerned about rule of three here, make the class noncopyable, or declare the copy assignment operator as private (and leave it undefined).

If that's not appropriate for your datatype, then that is an indication that those data members should not be const.

Intendance answered 7/9, 2010 at 13:10 Comment(0)
M
0

I think you are misunderstanding const correctness somewhat. If you really want an immutable structure, having an operator= does not make sense.

However, I fail to see why the code below doesn't fit your needs:

struct Vector
{
    Vector(float, float);
    Vector(Vector const&);

private:
    const float x, y;

    // Disable automatic generation of operator=
    Vector& operator=(Vector const&);
};
Melbamelborn answered 7/9, 2010 at 13:12 Comment(0)
W
0

This should do it.

Vector& Vector::operator=(const Vector& _rhs) {
    Vector tmp( _rhs.x, _rhs.y );
    std::swap( *this, tmp );
    return *this;
}
Winburn answered 7/2, 2012 at 14:38 Comment(1)
std::swap calls operator=, so it is an endless recursionSadler

© 2022 - 2024 — McMap. All rights reserved.