I am trying to create a GUI system with C++11 where, perhaps like Godot, every "node" has a very specific purpose such as grouping other nodes, drawing a rectangle, or detecting when a specific area of the screen is pressed down. Some parts of the code are getting a little repetitive as switch statements will determine a node's type and cast it appropriately, only to call the exact same function name in each case.
I thought I could clean this up by abstracting each node a bit more so the Rectangle
class would inherit not just the BaseNode
class, but also the Visibility
class to specify that it has a Draw()
function to call. By making Visibility::Draw()
a virtual function and overriding it in each derived class, I should be able to replace the long switch statement with a single line that casts the BaseNode*
to a Visibility*
and call Draw()
on that to display the rectangle, circle, image, polygon, or whatever specific functionality that individual node is programmed to have.
I wrote up this theoretically good solution yet found that it would call neither Rectangle::Draw()
nor Visibility::Draw()
but instead some random part of memory!
I eventually pinpointed the issue and wrote up this small demo to replicate it. Casting pRect
to a Visibility*
and calling the Draw()
function will execute Rectangle::BasicFunc()
inherited from BaseNode
instead of Rectangle::Draw()
inherited from Visibility
.
#include <iostream>
class BaseNode {
public:
virtual void BasicFunc() {
std::cout << "BaseNode BasicFunc()" << std::endl;
}
};
class Visibility {
public:
virtual void Draw() {
std::cout << "Visibility Draw()" << std::endl;
}
};
class Rectangle : public BaseNode, public Visibility {
public:
void BasicFunc() {
std::cout << "Rectangle BasicFunc()" << std::endl;
}
void Draw() {
std::cout << "Rectangle Draw()" << std::endl;
}
};
int main() {
BaseNode* pRect = new Rectangle;
((Visibility*)pRect)->Draw(); // Calls Rectangle::BasicFunc() instead of Rectangle::Draw()
return 0;
}
Some other testing I've done:
pRect
to a Rectangle*
instead will correctly call Rectangle::Draw()
, but then it's back to the bulky switch statement.virtual
keyword from Visibility::Draw
will make the Draw()
call execute Visibility::Draw()
. Correct function name but not the correct class.Rectangle
class does not matter.pRect
as a BaseNode*
will make BasicFunc()
always work.pRect
as a Visibility*
will make Draw()
always work.pRect
from a BaseNode*
to a Visibility*
will make Draw()
always fail and call BasicFunc()
instead. (The current issue)pRect
from a Visibility*
to a BaseNode*
will make BasicFunc()
always fail and call Draw()
instead.pRect
as a void*
will make both BasicFunc()
and Draw()
call the function associated with Rectangle
's first inherited class.I am completely unsure how to go about fixing this. Any advice is appreciated!
Running with address sanitizer indicates the problem immediately:
runtime error: member call on address 0x602000000010 which does not point to an object of type 'Visibility'
The issue here is that you cannot directly cast a BaseNode
object to a Visibility
object because there is no such relationship between these two classes. What you have here is undefined behavior.
Because a static_cast
is impossible here, the compiler resorts to a reinterpret_cast
, which in this case leads to you invoking the virtual table of a different class from the one the compiler thinks it's using.
One possible workaround is to use use dynamic_cast
:
auto pVisibility = dynamic_cast<Visibility*>(pRect);
if (pVisibility) {
pVisibility->Draw();
}
However, using this can often be seen as a "code smell". There are various other approaches you may use, but that's entirely up to your other design considerations.
As an aside, you should add a virtual destructor to BaseNode
, because right now trying to delete pRect;
has the potential to cause undefined behavior:
class BaseNode {
public:
virtual ~BaseNode() = default;
virtual void BasicFunc() {
std::cout << "BaseNode BasicFunc()" << std::endl;
}
};