Jump to content

  • Log In with Google      Sign In   
  • Create Account


Rule of Three, and const-correctness questions


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
39 replies to this topic

#1 johnmarinelli   Members   -  Reputation: 304

Like
4Likes
Like

Posted 05 February 2013 - 03:37 PM

Hi guys,

 

I was recently told that my code is missing the Rule Of Three concept, and that my timer isn't const-correct.

 

I know of these terms; but google and searching the forums didn't help much.

 

I think what would help, however, would be an example of each concept (Rule of Three, Const-correctness) related to Game Programming.  I know what the Rule Of Three is for (creating copy ctor, assignment ctor, and dtor), but I don't completely understand why.  Also, I know that RAII/smart pointers take care of all that altogether, but I'd like to understand the Rule of Three.

 

As far as const-correctness, I think that it means that when you pass a variable by reference to a function, you should pass it as a const so that you're assuring it doesn't get changed.  does this mean that in my game loop, the Update(int &deltaTicks) function should be Update( const int &deltaTicks )?


Edited by johnmarinelli, 05 February 2013 - 05:36 PM.


Sponsor:

#2 Radikalizm   Crossbones+   -  Reputation: 2777

Like
9Likes
Like

Posted 05 February 2013 - 04:04 PM

The reason for the C++ rule of three is that it's highly possible that if you had to implement your own copy-constructor, destructor or copy assignment operator because you needed different behaviour from the compiler-generated versions, the other 2 functions won't suit your needs either. 

 

An example

class Foo
{

public:
  
  Foo()
  {
    m_bar = new Bar();
  }

  ~Foo()
  {
    delete m_bar;
  }

private:

  Bar* m_bar;
  
};

 

In this example the default compiler-generated copy constructor or copy assignment operator would just copy the given pointer over to the destination instance. This would cause undefined behaviour once the destructor of a copy instance of Foo would get called.

 

 

The issue of const-correctness goes much deeper than just function arguments, it also applies to class methods (does a certain method change class data, or not?), return values, etc.

The easiest way to think of it is that if your data should remain immutable you should declare it as const. If a method operating on class data does not change this data, it should be declared const as well.

 

class Foo
{
public:

  // We declare this method as const, since it doesn't modify any data
  int getSomething() const { return m_something; }  

  // This method modifies our variable, so it shouldn't be declared as const
  void modifySomething(int n) { m_something += n; }

private:

  int m_something;   
};

 

 

To address your example about your Update function, passing a const argument by value is not what you're looking for. When you pass an argument by value, its contents will get pushed on the stack, so if you pass a variable as an argument to this function, the original variable will not get modified.

 

This does become an issue however when working with references. Try to find out for yourself what could happen to the arguments of type Bar in the following methods:

 

class Foo
{
public:

  void doSomething(const Bar& b);
  void doSomething(Bar& b);
};

I gets all your texture budgets!


#3 Aliii   Members   -  Reputation: 1345

Like
0Likes
Like

Posted 05 February 2013 - 04:08 PM

Ive met a few programmers but I never heard of Rule of Three.

Using consts makes more sense. Yes, it should be Update( const int deltaTicks ), ....but no big deal if you dont use const, but its highly recommented if other people are working on that code too. And you dont have to pass an int by reference when you want it to be const.



#4 leeor_net   Members   -  Reputation: 306

Like
2Likes
Like

Posted 05 February 2013 - 04:28 PM

Ive met a few programmers but I never heard of Rule of Three.

Using consts makes more sense. Yes, it should be Update( const int deltaTicks ), ....but no big deal if you dont use const, but its highly recommented if other people are working on that code too. And you dont have to pass an int by reference when you want it to be const.

 

Using const in the Update() function like that is useless in this case. The int parameter is being passed by value, not by reference. By value doesn't need a const qualifier because the value is copied, not referenced. You only need a const qualifier if you're passing by reference/pointer and you don't want the function to change the object being passed.

 

So to answer the OP's last question directly, no. You're not passing by reference there, you're passing by value so the const qualifier is useless.


Edited by leeor_net, 05 February 2013 - 04:31 PM.

-The Legend of Mazzeroth-Project Lead, Lead Software Architect & Engineer, General Tasks and Professional Chef.

 

http://www.lairworks.com


#5 Aardvajk   Crossbones+   -  Reputation: 5300

Like
4Likes
Like

Posted 05 February 2013 - 04:41 PM

Using const on an int as a parameter isn't useless - it's documenting. That said, it's not common as it is really only documenting. Using const on a return value that is passed by value is useless as the value can only be copied from, not assigned to, in any case.



#6 Hodgman   Moderators   -  Reputation: 27930

Like
5Likes
Like

Posted 05 February 2013 - 04:43 PM

I know what the Rule Of Three is for (creating copy ctor, assignment ctor, and dtor), but I don't completely understand why.

The rule is that if you have 1 of those things (a copy ctor, assignment op, or dtor), then you very probably need to have all 3 of them in order to avoid bugs.

Radikalizm's first example is a good one -- it relies on the destructor to clean up resources for it... however, that's 1 of the 3 things, with the other 2 missing. If someone copies/assigns that class, then the automatically-generated copy-ctor/assignment-op aren't going to do the right thing (which is to use new to create their own m_bar object).

 

There's a wikipedia page on the rule here: http://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)

 

Regarding const correctness, the C++ FAQ does a good job of explaining here: http://www.parashift.com/c++-faq/const-correctness.html



#7 johnmarinelli   Members   -  Reputation: 304

Like
0Likes
Like

Posted 05 February 2013 - 05:21 PM

This does become an issue however when working with references. Try to find out for yourself what could happen to the arguments of type Bar in the following methods:

 

Well, it looks like in the first example, b won't get changed where in the second example it could get changed.  I guess I'm overthinking this const-correctness thing smile.png

However, I (think) I still don't understand the uses of the copy ctor/assignment op;

class Foo{
private:
    int data
public:
/*normal ctor*/
    Foo(int d) : data(d) {};
/*copy ctor*/
    Foo(const &Foo copy) : data(copy.data){};
/*assignment op*/
    Foo& operator=(const &Foo copy){ data = copy.data };
};

is the above correct? 

if so, what do they have to do with dynamically allocated memory?

Foo *_ptr = new Foo();

/*does this create a whole new space in memory for test?
  if it does, am I correct in saying that the default assign op
  only makes test point to the same area of memory as _ptr - hence the whole
  deal about copy assignment operators?*/
Foo test = *_ptr;

Edited by johnmarinelli, 05 February 2013 - 05:46 PM.


#8 Aliii   Members   -  Reputation: 1345

Like
0Likes
Like

Posted 05 February 2013 - 05:28 PM

Ive met a few programmers but I never heard of Rule of Three.

Using consts makes more sense. Yes, it should be Update( const int deltaTicks ), ....but no big deal if you dont use const, but its highly recommented if other people are working on that code too. And you dont have to pass an int by reference when you want it to be const.

 

Using const in the Update() function like that is useless in this case. The int parameter is being passed by value, not by reference. By value doesn't need a const qualifier because the value is copied, not referenced. You only need a const qualifier if you're passing by reference/pointer and you don't want the function to change the object being passed.

 

So to answer the OP's last question directly, no. You're not passing by reference there, you're passing by value so the const qualifier is useless.

 

"Using const in the Update() function like that is useless in this case"

...well, he said its a reference:), I thought he just forgot the &

 

"You only need a const qualifier if you're passing by reference/pointer and you don't want the function to change the object being passed."

...remove the word only from this sentence and it will be correct.



#9 Ravyne   Crossbones+   -  Reputation: 6779

Like
1Likes
Like

Posted 05 February 2013 - 05:30 PM

You only need a const qualifier if you're passing by reference/pointer and you don't want the function to change the object being passed.


In addition to documenting your intent, const does have a function in this case -- it prevents you from modifying the argument inside the function body. I know it might seem useless to enforce rules on yourself, but this kind of defensive coding helps you to not make trivial mistakes, and prevent future code maintainers from violating your assumptions -- e.g. if your implementation assumes the argument to be constant, but you don't mark it as such, and someone comes along later to fix a bug and modifies it.

I would, and in fact do, go so far as to pass all arguments as const by default, regardless of whether they are by value or by reference. In most cases I would even go one further and take a local copy of that const parameter (in the smallest possible scope) if I wanted to modify it. One place I *don't* do this is when I mean to enable the RVO (the Return Value Optimization), as I do when implementing operator+ in terms of operator+= in my classes.

Edited by Ravyne, 05 February 2013 - 05:32 PM.


#10 Hodgman   Moderators   -  Reputation: 27930

Like
4Likes
Like

Posted 05 February 2013 - 08:57 PM

However, I (think) I still don't understand the uses of the copy ctor/assignment op;

class Foo{
private:
    int data
public:
/*normal ctor*/
    Foo(int d) : data(d) {};
/*copy ctor*/
    Foo(const &Foo copy) : data(copy.data){};
/*assignment op*/
    Foo& operator=(const &Foo copy){ data = copy.data };
};
is the above correct?

Well, it behaves as you'd expect... except the assignment operator should be:
Foo& operator=(const &Foo copy){ data = copy.data; return *this; };
It breaks the rule of 3 though -- either you should have 0 of the 3 things in the rule, or 3 of the 3 things. You've only written 2 of them. The rule is: if you have a copy-ctor, assignment-op or destructor, you should have all three of them.

However, the copy-ctor and assignment-op aren't needed for such a simple class -- the compiler will generate them if you don't write them yourself, so that class should just be:

class Foo{
    int data
public:
    Foo(int d) : data(d) {};
};

This also satifies the rule, because you have 0 out of 3 of those things now.

if so, what do they have to do with dynamically allocated memory?

In your example, the only member is an int, so no custom copy/assignment/destructor logic is required.
If your class manages resources (like dynamic memory), then it now needs all 3 of them:

class Foo{
private:
    int* data
public:
/*normal ctor*/
    Foo(int d) : data( new int(d) ) {}; //the default would not initialize data at all, we want to allocate some memory
/*copy ctor*/
    Foo(const &Foo copy) : data( new int(*copy.data) ){};
   // the default would be: Foo(const &Foo copy) : data(copy.data)
   // we instead want to allocate a new int, and copy the other's value into our new one
/*assignment op*/
    Foo& operator=(const &Foo copy) // the default would be: data = copy.data; return *this;
    {//we don't want copy the address/pointers, we need to copy the value located at one pointer to the variable pointed to by the other
        *data = *copy.data;
        return *this;
    }
/*destructor*/
    ~Foo() { delete data; } //the default would do nothing, we need to clean up our memory allocation
};

The above custom implementations ensure that every Foo owns it's own int (that is created with new) and also cleans up after itself (using delete in the destructor).
If you let the compiler implement those 3 functions for you, it would simply copy the pointer around the place, and multiple Foo objects would end up sharing the same int object.

Foo *_ptr = new Foo();
/*does this create a whole new space in memory for test?
  if it does, am I correct in saying that the default assign op
  only makes test point to the same area of memory as _ptr - hence the whole
  deal about copy assignment operators?*/
Foo test = *_ptr;

This is a whole different question, and shows you need to first learn about stack vs heap allocations (or automatic storage vs free-store storage in strict C++ terminology).

When you write: int foo = 42; the variable 'foo' is created in the call-stack. It's a "temporary variable" that only exists for the lifetime of the function. When the function returns, 'foo' disappears.

When you write int* foo = new int(42), the variable 'foo' is a temporary on the call-stack as before, but this variable just stores the address of a memory allocation elsewhere. It's your responsibility to ensure that memory allocation is cleaned up with delete.

You can use complex types instead of int and everything works the same, except that when you create the variable, the constructor is called, and when the function returns, the destructor is called.

void test1()
{
  Foo test;//memory allocated on the stack, constructor is called;
  return;//destructor is called, stack is unwound
}
void test2()
{
  Foo* test;//memory for 'test' (the address/pointer variable) on the stack.
  test = new Foo();//Memory for a Foo object created on the heap, constructor is called, address of allocation stored in test
  delete test;//destructor called, heap allocation is cleaned up
  return;//stack is unwound
}
void test3(Foo* input)//this argument contains the address of a Foo object
{
  Foo test;//default constructor called, object exists on the stack
  test = *input;//assignment operator is called, which should copy the values located at the input allocation into the stack allocation at &test
  return;//test's destructor is called
}

Edited by Hodgman, 05 February 2013 - 09:03 PM.


#11 King Mir   Members   -  Reputation: 1916

Like
0Likes
Like

Posted 05 February 2013 - 10:47 PM

However, I (think) I still don't understand the uses of the copy ctor/assignment op;

class Foo{
private:
    int data
public:
/*normal ctor*/
    Foo(int d) : data(d) {};
/*copy ctor*/
    Foo(const &Foo copy) : data(copy.data){};
/*assignment op*/
    Foo& operator=(const &Foo copy){ data = copy.data };
};

is the above correct?

 

It's bad style, because the default copy constructor and assignment operator do the same thing. Typically the reason you need to overload these functions is when you are writing a class that manages a resource, or is a container.

 

 

Also, for managing resources, you can sometimes avoid writing the rule of three functions, by useing a smart pointer, like unique_ptr with a custom deleter.


Edited by King Mir, 05 February 2013 - 10:57 PM.


#12 Trienco   Crossbones+   -  Reputation: 2058

Like
1Likes
Like

Posted 05 February 2013 - 11:26 PM

Since const correctness still comes across as something that's "nice but not mandatory", let's look at things that will fail if you don't write const correct code.

 

First, the one thing that can really get me mad: people writing interfaces/APIs using char* instead of const char*.

void parseString(char* str) { ... }
 
std::string text;
parseString(text.c_str()); //Fail, as c_str() returns const char*

 

 

While these have a simple workaround, it can still be annoying

void analyseObject(MyClass& obj) { ... }
 
analyseObject( MyClass(a,b,c) ); //Fail, temporary variables are r-values and const

 

struct MyClass {
   void print() {...}
}
 
MyClass(a,b,c).print(); //Fail, temporary = const

 

 

To clarify the observable effects of ignoring the Rule of Three, because unlike missing const correctness, this won't conveniently fail to compile in the first place:

 

struct MyClass
{
    int* data;
    MyClass() : data(new int) {}
    ~MyClass() { delete data; }
};
 
 
vector<MyClass> v;
v.push_back(MyClass()); //Fuse is lit
cout << v.front().data; //Fail
v.clear(); //Fail
 
void function(MyClass a) { ... }
 

 

Let's ignore compilers optimizing out the temp variable or using move semantics, since you can't rely on it.

 

First example creates a temporary instance, then creates a copy in the vector (which just stupidly copies the pointer). The temporary goes out of scope and deletes data. The next line accesses data that was just deleted. You will see either garbage (kind of helpful), crash with an access violation (very helpful) or it will just seem to "work fine" (dangerous), as the memory will most likely not have been overwritten yet.

 

Then we clear the vector and delete the instance in it. Now data is deleted a second time. If you are lucky, your application will crash.

 

 

The second example looks harmless, but does pretty much the same. To call the function, you create a copy. When the function returns, this copy is destroyed and deletes your data. The original object is now in an invalid state with data pointing to deleted data. Any access will hopefully crash and when the original object goes out of scope, the memory is attempted to be deleted a second time.

 

 

Why is crashing the best case scenario? Because it is hard to miss and clearly tells you there is a bug. The same bug could result in working "just fine" most of the time, but returning strange values on every odd Thursday during full moon. This can cost you days or weeks just trying to reproduce the bug and tracking it down.


Edited by Trienco, 05 February 2013 - 11:27 PM.

f@dzhttp://festini.device-zero.de

#13 L. Spiro   Crossbones+   -  Reputation: 12452

Like
5Likes
Like

Posted 06 February 2013 - 12:07 AM

The Rule of Three is out-dated and does not play well with exceptions. The Rule of Two is much safer as it relies on RAII, which is in itself (almost) always the correct choice.
Rule of Two

 

 

L. Spiro


It is amazing how often people try to be unique, and yet they are always trying to make others be like them. - L. Spiro 2011
I spent most of my life learning the courage it takes to go out and get what I want. Now that I have it, I am not sure exactly what it is that I want. - L. Spiro 2013
I went to my local Subway once to find some guy yelling at the staff. When someone finally came to take my order and asked, “May I help you?”, I replied, “Yeah, I’ll have one asshole to go.”
L. Spiro Engine: http://lspiroengine.com
L. Spiro Engine Forums: http://lspiroengine.com/forums

#14 iMalc   Crossbones+   -  Reputation: 2264

Like
1Likes
Like

Posted 06 February 2013 - 12:13 AM

Hodgman, on 06 Feb 2013 - 15:44, said:
The rule is: if you have a copy-ctor, assignment-op or destructor, you should have all three of them.

I'd make a few minor but important alterations to that wording:
"if you NEED a copy-ctor, assignment-op or destructor, you should PROBABLY have all three of them."

The addition of the word "need" is because it is entirely possible that you have one or more of the three implemented, but in fact do not need any of them. Post #7 is a great example of this. As the default auto-generated methods already do the right thing, implementing them yourself is an extra unnecessary source of potential bugs, and is likely less-efficient. It's really the wrong thing to do. You simply don't need either the copy-constructor and/or assignment operator in this case.
I've come across many bugs that resulted from an assignment operator that was always completely unnecessary, and because it wasn't updated when another member variable was added to the class, it caused a bug. If it was left to the compiler to auto-generate then the bug would never have existed.

Secondly, the addition of "probably". Whilst this does seem to lessen the strength of the statement, in practice it's a very strong "probably". There are very rare cases where one might need one or two of them but not all three, but you won't run into those until well after you've got the rule of three completely nailed. I wont bother to explain such cases because when you're experienced enough you WILL know them, and it is only then that you should care about them.
Also (and this was meant to be my main reason), whilst it is still typically "wrong" to implement one but not all three of them, often there is another way of following the rule, and that is by explicitly disabling the copy-constructor and/or assignment operator by making them private and leaving them unimplemented. Or it can be done by deriving from a non-copyable class, or having a reference as a member...
Basically so long as the class either: can be copied and will do so correctly, or it CANNOT be copied, then it is alright.
"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms

#15 Strewya   Members   -  Reputation: 1171

Like
1Likes
Like

Posted 06 February 2013 - 02:07 AM

I have to agree that the rule should be the Rule of Two. If you need a unique assignment or copy ctor, then you really need both of them. If you have smart pointers (and you should, instead of raw pointers) or const members, the autogenerated destructor does the job right, but the assignment and copy ctor need to be custom.


devstropo.blogspot.com - Random stuff about my gamedev hobby


#16 King Mir   Members   -  Reputation: 1916

Like
0Likes
Like

Posted 06 February 2013 - 09:12 AM

The rule of two really just underscores that each object should manage at most one resource. If you need an object to manage more than one resource, then put each resource in it's own wrapper. But for the object managing a single resource, you need the rule of three. Or a smart pointer with the right copy semantics.

 

So I would say no to the rule of two.



#17 jwezorek   Crossbones+   -  Reputation: 1661

Like
2Likes
Like

Posted 06 February 2013 - 10:53 AM

Steps to writing const-correct code:
 
1. Understand what const variables and const references & pointers actually mean and how they behave at compile time. Read about it. Make a sample program and test what happens when you try to do various things.
 
2. Understand what a const member function is. As above...
 
3. In your actual code, when you write a new function or member function attempt to make all of its parameters const, especially pointers and refences. Deal with the syntactic fallout.
 
4. Where 3. turned out to be impossible realize that those parameters can't be const.
 
5. Do this for a while and it will all become second nature.

Edited by jwezorek, 06 February 2013 - 10:56 AM.


#18 Ravyne   Crossbones+   -  Reputation: 6779

Like
2Likes
Like

Posted 06 February 2013 - 01:15 PM

Oh, and while we're talking about const, let us not forget that C++11 const is not your father's const [Herb Sutter]. If you don't already know why that is, go watch the video immediately.



#19 Khatharr   Crossbones+   -  Reputation: 2872

Like
0Likes
Like

Posted 06 February 2013 - 06:55 PM

I come to agree about using const wherever possible. Whenever I see a function accepting a non-const argument that's not simply a value I always want to know what it's going to change. This is really important in public interfaces, since even if the designer doesn't intend to change something passed as non-const there could be bugs in the code that change it anyway. It's worthwhile to pass values as const as well since const arguments clearly represent state external to the function rather than being just another variable in the function to be freely modified. I'm not saying that you can't pass by value and then modify the value if you really must, but if you're not doing so then const will help guard against bugs.

 

I'm curious about that video but I'm working with bandwidth limits here. Is there a print source that describes the change? Googling "C++11 const changes" didn't get me anywhere interesting.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#20 King Mir   Members   -  Reputation: 1916

Like
0Likes
Like

Posted 06 February 2013 - 08:03 PM

I'm curious about that video but I'm working with bandwidth limits here. Is there a print source that describes the change? Googling "C++11 const changes" didn't get me anywhere interesting.

 

Basically Sutter says that the standard library can only guarantee race condition free, and therefore well defined, use if all const parameters passed to it are never modified, or are internally synchronized. He therefore says that if you do need to modify the state of a const object, you should only do it though internally synchronized objects that are declared mutable. He goes on to say that mutable should be used by default on all internally synchronized objects, like atomics and mutexes.

 

Therefore "const" means "thread safe", and "mutable" means "internally synchronized".

 

This is a more stringent constraint on const than implied by the previous standard and Sutter says that any code using const_cast or mutable should be examined in light of the change.


Edited by King Mir, 06 February 2013 - 08:09 PM.





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS