🔒 Locked

const Madness

Started by Ashkan Feb 27, 2008 at 4:29 AM 21 replies 2.1k views

Original Post

Feb 27, 2008 at 4:29 AM

OK, here's the situation: I have a virtual member function with its const overload in a Base class and a Derived class in which that virtual function is overridden:


struct Type {
};

class Base {
public:
	virtual const Type& Get() const = 0;
	              Type& Get()       { return const_cast< Type& >(static_cast< const Base& >(*this).Get()); }
};

class Derived : public Base {
public:
	virtual const Type& Get() const { return type; }

private:
	Type type;
};

Now, this code works as expected as long as a Derived object is pointed to by a Base pointer, but as soon as this Derived object is pointed to by a Derived pointer, the second, non-const function is hidden, meaning that all non const objects of Derived fail to call Get()?! So,


// This works fine
Base* pBase = new Derived;
Type& type = pBase->Get();

// This fails to compile
Derived* pDerived = new Derived;
Type& type = pDerived->Get();

// But then this works fine
Derived* pDerived = new Derived;
const Type& type = pDerived->Get();

So at first I thought it has something to do with the Derived Get() function hiding the non virtual, non-const Get() in the base, but astonishingly even making the second non-const function virtual won't help a bit which strikes me as odd. The only solution that I've found to make this code work is to somehow redefine the function in the Derived class, be it by redefining the virtual function in the derived class or simply making it pure virtual in an effort to force all subclasses implement it. Now why is that so?! Thanks

ToohrVyk

Feb 27, 2008 at 4:36 AM

The compiler looks for a member function named Get. Since there is one inside the Derived class, it stops there without looking at those found inside Base. Then, it looks for a non-const overload in that class, and finds none, which causes an error.

You must include the Get functions from the Base class inside Derived:

class Derived : public Base {public:	using Base::Get;	virtual const Type& Get() const { return type; }private:	Type type;};

Ashkan

Feb 27, 2008 at 4:41 AM

Thanks for the quick answer Victor. I certainly expected a better work on the compiler's part though.

ToohrVyk

Feb 27, 2008 at 4:48 AM

Don't worry. With time, you will stop expecting sane behavior from the C++ language.

Rydinare

Feb 27, 2008 at 6:41 AM

One stylistic thing, any reason why you have your non-const version of Get calling the const version and then casting away const? Wouldn't you be better served to do the reverse, thus avoiding the extra casts?

ToohrVyk

Feb 27, 2008 at 6:51 AM

Rydinare is right. As it is written right now, your base class only works if all derived class implementations of the virtual function are guaranteed to return a const reference to a non-constant object, which is an extremely difficult guarantee to uphold (for instance, your example Derived class does not provide that guarantee).

Koen

Feb 27, 2008 at 7:08 AM

That wouldn't work because you cannot call a non-const method inside a const method.
Whenever I need both getters, I do this
const X& Y::GetX() const{ return x;}X& Y::GetX(){ return x;}

Does anybody have a good way of solving this code duplication?
edit: nevermind. I should read more carefully before I post :) I thought you were suggesting to do it without const_cast, while you meant using only one cast instead of two.
So you think it's ok to use a const_cast in these situations? No better solutions?

Nitage

Feb 27, 2008 at 7:18 AM

Quote:
That wouldn't work because you cannot call a non-const method inside a const method.

You need to const_cast the this pointer.

samoth

Feb 27, 2008 at 7:59 AM

Would you mind explaining the advantage of this technique over a "normal" pure virtual function, please?
No seriously, don't laugh at me... I've been staring at it for half an hour, but I don't understand what it does (other than what a pure virtual function does, too).

Koen

Feb 27, 2008 at 8:06 AM

Quote:
Original post by samoth
Would you mind explaining the advantage of this technique over a "normal" pure virtual function, please?
No seriously, don't laugh at me... I've been staring at it for half an hour, but I don't understand what it does (other than what a pure virtual function does, too).

The point is to only implement each getter once. In Ashkan's implementation the const getter is pure virtual. The non-const one has a base-implementation that uses the const getter. So all deriving classes only have to implement the const one. If they both were pure virtual, you'd have to implement both getters in the deriving classes.
I'm still not convinced this is a good thing to do, though.

Rydinare

Feb 27, 2008 at 8:28 AM

Sorry for being a little vague in my first post. That's what I get for making a quick post while my snuggly cat is distracting me and I'm getting ready to head out the door. [grin]

I was suggesting having the non-const version be the pure virtual and having the const-version call it in the base, instead of the reverse. I just think the code is a bit cleaner and more intuitive this way. Anyway, here's the modified code with my suggestion:

struct Type {};class Base {public:	virtual Type& Get() = 0;	const Type& Get() const	{ 		return const_cast<Base&>(*this).Get();	} // end Get};class Derived : public Base {public:	using Base::Get;	virtual Type& Get() { return type; }private:	Type type;};

Koen

Feb 27, 2008 at 8:39 AM

Quote:
Original post by Rydinare
I was suggesting having the non-const version be the pure virtual and having the const-version call it in the base, instead of the reverse. I just think the code is a bit cleaner and more intuitive this way.

Then ToohrVyk's comment still holds. A stupid example:

class Derived : public Base {public:	using Base::Get;	virtual std::string& Get() { s.clear(); return s; }private:	std::string s;};

The base implementation of 'Get() const' now performs a non-const operation on a const data-member.

Rydinare

Feb 27, 2008 at 9:04 AM

Quote:
Original post by Koen
Quote:
Original post by Rydinare
I was suggesting having the non-const version be the pure virtual and having the const-version call it in the base, instead of the reverse. I just think the code is a bit cleaner and more intuitive this way.

Then ToohrVyk's comment still holds. A stupid example:
class Derived : public Base {public:	using Base::Get;	virtual std::string& Get() { s.clear(); return s; }private:	std::string s;};
The base implementation of 'Get() const' now performs a non-const operation on a const data-member.

Hmm... Interesting. I might argue that that would be a semantic abuse of an accessor. But, you definitely have a point. Nothing prevents it.

A better question might be: does the trouble of code reuse in this case outweigh the ugliness of the extra casts? In this case, you might consider the answer to be no and choose to make both the const and non-const gets pure virtual.

Sneftel

Feb 27, 2008 at 9:25 AM

Quote:
Type& Get() { return const_cast< Type& >(static_cast< const Base& >(*this).Get()); }

Do not do this. It's unsafe. It can invoke undefined behavior. It loses you the benefit of covariant return types.

Just keep two virtual functions. It's not going to break the bank.

Koen

Feb 27, 2008 at 9:31 AM

Quote:
Original post by Rydinare
Hmm... Interesting. I might argue that that would be a semantic abuse of an accessor.

True. But that stuff happens all the time on large projects ;-)

Rydinare

Feb 27, 2008 at 12:24 PM

Actually, come to think of it, this brings forth a deeper issue. In reality, you really don't want to provide non-const accessors to your data members in most cases (there are some cases where it's valid), because you're reducing encapsulation. Instead, you should probably provide methods that allow you to do things to the data member.

For example:

class MyClass{public:   const Type& get() const;   void doStuff() { ... some action is performed on myType_ ... }   ... etc ...private:   Type myType_;};

If you can avoid providing any accessors, you're in even better shape. I only provide accessors if they're absolutely essential, which is generally the minority of cases.

Ashkan

Feb 27, 2008 at 2:23 PM

Sorry for the late answer. I was out all day so I couldn't answer earlier.

First of all thanks for all your comments. As it turns out there's quite some confusion surrounding the way I'm handling things here, so an explanation is in order. The reason I'm doing this is to avoid code duplication! Code size reduction is a minor issue here ans is only part of the reason. Think about maintenance time, increased compilation times and code-bloat headaches. But probably, the most important reason is to keep all the code in one place so changes in the code are centralized. Yes, casting the const away is usually a very bad idea, but why not use it under very controlled circumstances, when it helps out . Maybe all these reasons still don't justify the effort (and I kind of have this feeling to be honest) but it's one those things that you pick up as a habit. Surprisingly, I read about this trick in the third item of Meyers' Effective C++, third ed., about a year ago. I like his books, so I kind of picked up the habit without questioning his motives and I haven't had any problems with that (mostly because all functions are accessed through the interfaces) up until last night.

Quote:
Original Post by ToohrVyk

So, why not do things the other way around you ask? The reason is simple. What if the non-const version modifies the object? Here's an example.

struct Type {};class SomeClass {public:    const Type& F() const { return const_cast< SomeClass& >(*this).F(); }    Type& F() { ++var; return type; }private:int var;Type type;};

Quote:
Original Post by Rydinare
Actually, come to think of it, this brings forth a deeper issue. In reality, you really don't want to provide non-const accessors to your data members in most cases (there are some cases where it's valid), because you're reducing encapsulation. Instead, you should probably provide methods that allow you to do things to the data member.

Actually I've come across suggestions of this nature a couple of times, but my question to advocates of this approach is this: How stuffing a class with responsibilities, that should have in fact been assigned to several classes can be considered a good design? This only results in big fat classes that are trying to fulfill the responsibilities of several minor classes, all on their own. If that's not the way to do things then, I assume by suggesting this approach you mean that such classes should provide functions of this nature, but delegate the call to the appropriate internals, in which case, I have to say what's the point in doing that? But maybe I'm missing something here. So please advise. I really appreciate an example here to clear things up once and forever. How do you go about designing a class that doesn't require getters, or at least non-const getters? It should bring up an interesting discussion in my opinion.

Quote:
Original post by Sneftel
Do not do this. It's unsafe. It can invoke undefined behavior. It loses you the benefit of covariant return types.

How's that so? Would you mind illuminating some light on this?

Rydinare

Feb 27, 2008 at 2:35 PM

Quote:
Original post by Ashkan
Quote:
Original Post by Rydinare
Actually, come to think of it, this brings forth a deeper issue. In reality, you really don't want to provide non-const accessors to your data members in most cases (there are some cases where it's valid), because you're reducing encapsulation. Instead, you should probably provide methods that allow you to do things to the data member.

Actually I've come across suggestions of this nature a couple of times, but my question to advocates of this approach is this: How stuffing a class with responsibilities, that should have in fact been assigned to several classes can be considered a good design? This only results in big fat classes that are trying to fulfill the responsibilities of several minor classes, all on their own. If that's not the way to do things then, I assume by suggesting this approach you mean that such classes should provide functions of this nature, but delegate the call to the appropriate internals, in which case, I have to say what's the point in doing that? But maybe I'm missing something here. So please advise. I really appreciate an example here to clear things up once and forever. How do you go about designing a class that doesn't require getters, or at least non-const getters? It should bring up an interesting discussion in my opinion.

I'm not sure why the view is "stuffing" the class with responsibilities. I'm not saying that if your class contains a string that it should expose everything you can do with a string. On the contrary, I'm saying that the only way to do anything to the internal string would be in such a way that is consistent with the purpose of the class. The point is that the internal member can't be changed without a purpose that is consistent with the expectations of the class.

For example, if there's a bottle class, someone shouldn't be able to add water without removing the cap. If you're allowing access to the internals, that's exactly what can happen. e.g.:

Bottle bottle;bottle.open();bottle.addLiquid(water);

Instead of:

Bottle bottle;bottle.getLiquid() = water; // water added without opening the bottle

Ashkan

Feb 27, 2008 at 3:21 PM

That example doesn't do it justice. Let's say we have a Mesh class. A Mesh has a pointer to a Material object as a member. Shouldn't we let users of this class access that Material with a getter? I think we both agree that a const getter is unavoidable, but what about a non-const getter? Shouldn't client programmers be able to change the visual properties of this Mesh? The only [unintuitive] approach that I can think of besides getters is to duplicate Material's interface into the Mesh and so users of this class would call ChangeDiffuseColor( const Color& newColor ) for instance, to change the color of this mesh, while this function internally delegates this call to something like pMaterial->ChangeDiffuseColor( newColor ).

This is where I choose to provide both the const and non-const versions of the getters in order to provide
1) The flexibility that's expected from a non-const Mesh object,
AND
2) The immutability benefits of a const Mesh object.

Rydinare

Feb 27, 2008 at 4:57 PM

Quote:
Original post by Ashkan
That example doesn't do it justice. Let's say we have a Mesh class. A Mesh has a pointer to a Material object as a member. Shouldn't we let users of this class access that Material with a getter? I think we both agree that a const getter is unavoidable, but what about a non-const getter? Shouldn't client programmers be able to change the visual properties of this Mesh? The only [unintuitive] approach that I can think of besides getters is to duplicate Material's interface into the Mesh and so users of this class would call ChangeDiffuseColor( const Color& newColor ) for instance, to change the color of this mesh, while this function internally delegates this call to something like pMaterial->ChangeDiffuseColor( newColor ).

This is where I choose to provide both the const and non-const versions of the getters in order to provide
1) The flexibility that's expected from a non-const Mesh object,
AND
2) The immutability benefits of a const Mesh object.

There's another option. What you're saying is that you can view it that a Mesh owns a Material and thus to access the Material, you need to ask the Mesh for it. That's one valid way to look at it. Another way is that they're separate entities and a client provides an already set up Material to a Mesh class. The Material provided is already set up and doesn't need to be changed. If the Material needs to be changed, you could think of it as being an entirely different Material. Therefore, you might call the Mesh::replaceMaterial() method which sets the Material to the new Material you've provided.

I also will agree that sometimes non-const accessors are difficult to avoid. In many cases, they are avoidable, without cluttering your public interface, though. I'll go a step further and say that I've generally found that not allowing non-const accessors has made my interfaces cleaner.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.