Sign in to follow this  
RunningInt

C++ A problem with polymorphism

Recommended Posts

The title is a bit vague because I don't actually know what this problem is called. I will give an example of the problem. Below is a simple shape class heirarchy. I have left a lot necessary things out I know. It is just an example:

class CShape
{
  virtual void dummy()=0;
};

class CSquare : public CShape
{
  void setRadius();
}

class CCircle : public CShape
{
  ..
}





Okay now say that I have a container of shape pointers and I fill it with various shapes:
  vector<CShape*> shapes;
  
  shapes.push_back(new CSquare());
  shapes.push_back(new CCircle());





Now here is the problem: What if I want to iterate through the shapes in that container and call the setRadius method on each circle? Ie:
for (int i=0;i<shapes.size();i++)
{
  shapes[i]->setRadius();
}





The problem is that not all the shapes have to be circles. Some might be squares. At run time I do not know. If one of the shapes is a square, then the call to setRadius() could fail. There are several options I can think of getting round this problem, and I was just wondering if anyone had any ideas. 1st solution Abstract class CShape defines a virtual member function called ID. Derived classes must implement this function to return a unique ID. Therefore I could do this:
for (int i=0;i<shapes.size();i++)
{
  if (shapes[i].ID() == CIRCLE)
    shapes[i]->setRadius();
}





2nd solution Use RTTI (which I only just found out existed) to determine the type of shape:
for (int i=0;i<shapes.size();i++)
{
  if (CCircle*cptr = dynamic_cast <CCircle*>(shapes[i]))
    shapes[i]->setRadius();
}





3rd solution Implement an empty setRadius function in the base CShape class, it is then optional whether derived classes override it, but at least it can be called by all shapes without failing.
class CShape
{
  virtual void dummy()=0;
  void setRadius(); //this class will be implemented as empty
};

for (int i=0;i<shapes.size();i++)
{
  shapes[i]->setRadius();
}



This is my least favorite method as it just seems wrong to me. Not all shapes have a radius property, so in my opinion it shouldn't be part of a base shape class. Perhaps someone has a better method of doing this? (this is what I hope). Just to note that the generic shape container is required, it is not an option to have seperate containers for each type of shape. Wouldn't it be nice if when calling setRadius() on an object, if that method didn't exist on that class then it was just not called? Thanks for reading

Share this post


Link to post
Share on other sites
You shouldn't want to do that in the first place. The basic idea behind storing a list of Shapes [rather than a list of Circles or Squares] is that you are stating that you only care about the common interface. If you care about the radius of your shapes, then you should maintain a list of Circles instead.

CM

Share this post


Link to post
Share on other sites
You seem to have a failure of abstraction in your program. The question becomes why are you calling setRadius()? Chances are when you perform the setRadius() call you're actually doing an more abstract operation over all the elements in the container. If that's the case then that abstract operation should be made a virtual member function of the CShape class and the setRadius() call be made part of the implementation of the CCircle class' version.

Share this post


Link to post
Share on other sites
Okay it seems my mistake is a matter of design rather than implementation, and lots of you are suggesting changes in implementation, and they do seem like valid changes in the case of the shapes example, but I am not actually working on a project to do with shapes - that was just an example for my question of which implementation was better.

What I am working on is a 3D Tile engine, and the problem I have is to do with tiles. So perhaps if I explain that instead it will be better.

The terrain map contains a 2D array of tiles. Each tile holds pointers to it's neighbouring tiles. A consequence of the model I am using is that edge tiles are not active parts of the map. This means they will not be drawn, and objects cannot move into them. They are only there as placemarkers for the edge of the map. So there are two types of tile: EdgeTiles, and TrueTiles. Truetiles must have a number of methods which EdgeTiles will not (such as draw()).

I should be able to do:

Tile* mytile = maptile[40][30];
Tile* myneighbour = mytile->neighbour[NORTH];
myneighbour->Draw();

But if myneighbour is an EdgeTile this will crash. I cannot know this in advance. I do not want to explicitly test the X and Y coordinates of the tiles to determine if they are edges.

So what would be the best class heirarchy for this? Perhaps EdgeTile should be a base class, and TrueTile should extend that base class. But I still have this problem with the heterogeneous container.

Share this post


Link to post
Share on other sites
how about having only one tile class, and having the edge tiles have NULL pointers to their non-existant neighbours.

your code would be like this


Tile* mytile = maptile[40][30];
Tile* myneighbour = mytile->neighbour[NORTH];
if( myneighbour != NULL )
myneighbour->Draw();

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
That code should not even compile, unless you are doing some casting that you didn't show us. You cannot call the draw method on the Tile class if it does not define it.

Share this post


Link to post
Share on other sites
I'd look into making neighbor not return a valid tile if there is no neighbor in that direction. Either a null pointer that you can check for, or a smart pointer containing an edge tile that dies when you stop using it. Then make Draw virtual to be a no-op for edge tiles or if(!this)

But as others say, base classes for heterogeneous containers are often best being simply interfaces.

Share this post


Link to post
Share on other sites
Why are edge tiles a distinct type? It seems as though the easiest solution is to just not assign those pointers at all, and drop the entire concept of an edge tile.

Tile* myneighbour = mytile->neighbour[NORTH];
if(myNeighbor != 0)
myneighbour->Draw();

CM

Share this post


Link to post
Share on other sites
Quote:
Conner McCloud answers: Why are edge tiles a distinct type? It seems as though the easiest solution is to just not assign those pointers at all, and drop the entire concept of an edge tile.


Quote:
Rip-off answers: how about having only one tile class, and having the edge tiles have NULL pointers to their non-existant neighbours.


Hopefully this can explain why I chose to do it this way:

Each corner of a tile is an independent height. The height of a corner is stored by the tile of which it is the top-left corner. That is each tile stores one height - the height of it's top left corner. The image below illustrates which tile controls the height of each corner:



Notice that the white corners are not top-left corners of any tile. The height of these corners tiles is therefore undefined.

Rather than making these corners a special case, I simply leave them as undefined. This does mean that the tiles they are part of cannot be drawn, or involved in any function requiring knowledge of the corner heights. Therefore such tiles are effectively fake tiles. They are only there to hold a height value necessary for the tiles adjacent to them.

Share this post


Link to post
Share on other sites
That's the way I have usually done it. I guess I was just curious to merge the height map array into the tile array. I forgot that height map arrays have dimensions [w][h] while the tile map they represent is [w-1][h-1]

Fortunately I am still designing it so I can change back to the method you suggested.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this