8

Suppose I have a class Foo that has a private pointer to a Bar:

class Foo
{
private:
  Bar * bar;

public:
  Foo () : bar (new Bar ())
  {}

  ~Foo ()
  {
    delete bar;
  }
};

If the pointer bar should never be reassigned to a different instance, then it makes sense to make the pointer itself const in order to stop me (or a maintainer) from doing so in the future:

private:
  Bar * const bar;

I like doing this wherever the opportunity arises.

If I then wanted to write a move constructor, it would look something like this:

Foo (Foo && f) :
   bar (f.bar)
{
  f.bar = NULL;  // uh oh; f.bar is const.
}

I can "make the error go away" either by casting away the constness of f.bar or by not making it const in the first place. Neither of which are things I want to do. I'd rather not remove the const completely because it's there for a reason. On the other hand, casting away constness rings alarm bells for me and is something I never usually do. My question is: is it considered acceptable practice to cast away constness within a move constructor like this? Is there a better way that I haven't considered?

I don't think my question is the same as this one: Use const_cast to implement the move constructor

Community
  • 1
  • 1
peterpi
  • 575
  • 2
  • 11
  • 7
    If you need to change `bar` then quite clearly it shouldn't be `const`. Think carefully about your design. – user657267 Oct 24 '14 at 09:36
  • It looks like your class should not be movable. – juanchopanza Oct 24 '14 at 09:37
  • 2
    Using const_cast here is undefined behaviour. – Neil Kirk Oct 24 '14 at 09:46
  • @NeilKirk: even if `f` is not const ? – Jarod42 Oct 24 '14 at 10:02
  • f.bar is const, that's all that matters. – Neil Kirk Oct 24 '14 at 10:22
  • 1
    @user657267 the *only* time I want to change it is in the move constructor, and only on the instance that is soon-to-be-deleted. It seems a shame that this counts as enough to stop me using const. – peterpi Oct 24 '14 at 11:39
  • I'm not a C++11 expert, but this seems to make perfect sense: The move constructor clears out the parameter. Assigning `f.bar` to `NULL` is perfectly defined because the only thing that will happen shortly is the destructor of `f` will execute `delete bar;` BTW would the offending line become: `const_cast(f.bar) = NULL;` ? – quamrana Oct 24 '14 at 11:42
  • 2
    possible duplicate of [Move constructor and const member variables](http://stackoverflow.com/questions/6317429/move-constructor-and-const-member-variables) – quamrana Oct 24 '14 at 11:47
  • Don't declare things const if they're not logically const. By all means pass by const if you don't want the caller to modify it. This means the rule is: Don't make members const, make the thing const. This is far more important now we have move semantics, put the const in the right place; on the object when you pass it, or on the member function, but not the member. – BenPope Oct 24 '14 at 18:57

3 Answers3

6

If you will have to change the pointer in some case (even if it's only a single case), then it probably shouldn't be const.

However, even putting this thought aside, using const_cast to remove constness from an object, and then using the result of the cast invokes undefined behavior. It is only safe to const_cast a variable that was originally not const.

SingerOfTheFall
  • 29,228
  • 8
  • 68
  • 105
  • 1
    More precisely, it is undefined behaviour to write to the result of the cast. You can still read from the result. – Zyx 2000 Oct 24 '14 at 11:41
  • I guess that this lesson is THE ONE THING I learned this year about the correct vs. undefined-behavior-provoking use of const_cast: "Don't ever modify a variable whose declaration is const." – paercebal Oct 24 '14 at 11:58
4

You are basically contradicting yourself.
You are saying "I don't want to change it in any circumstance", and the you're saying "I want to change it when i move an object".

So, if there is a scenario where it needs to be changed, then it's not a const, and you can feel comfortable removing it.

If you are determined that you want it const, you can add another boolean value that determines ownership (if you need to handle resource life-time). So the pointed object may be released only when a destructor that owns the object is called.

Yochai Timmer
  • 48,127
  • 24
  • 147
  • 185
  • I agree that I am contradicting myself. I don't want it to change in any circumstance, with the sole exception of the move constructor. I suppose what I'm saying is that it's a pity that JUST the move constructor is going to prevent me from using const. – peterpi Oct 24 '14 at 10:06
  • 3
    It shows that your design has a flaw. There is a case where that pointer can be changed. Example: `{ Foo f; Foo f2(std::move(f)); f.somefn(); }` Nothing inherently wrong with that code snippet, but somefn() was supposed to be able to rely on the value of bar being unchanged. You'd very clearly told the compiler (and future readers of your code) that once bar has been initialized, it will _never_ change. But because the object was moved-from, it did change. Thus violating the const-ness of the member. – Andre Kostur Oct 24 '14 at 13:52
  • @AndreKostur This is a great explanation, thank you. It shows that "adding movability" is more than just adding a move constructor. In all member functions the behavior of an object *post move* needs to be considered. – peterpi Oct 24 '14 at 15:20
3

If possible, use unique_ptr instead:

std::unique_ptr<Bar> bar;
Foo(Foo&& f) : bar(std::move(f.bar)){}
// or maybe you won't even have to declare move constructor

This way not only you'll get more safety and comfort, but also you won't accidentaly rewrite it with =, you'll have to call .reset() so you'll think twice before doing it (which is your aim, I suppose).

Anton Savin
  • 40,838
  • 8
  • 54
  • 90