Polymorphism: trying to access subclass from superclass vector

[jkwok678]

Right now this is my code
I have a vector of Shared_ptr of Elements.
SignalTrack and StraightTrack inherit from Track.
Track inherits from Element. SignalTrack has a special property called aspect which others don't have. How can I access this property when iterating through the vector.

  std::vector<Shared_ptr<Element>> elementList;
  std::shared_ptr<SignalTrack> signalTrack(new SignalTrack(...,...,...));
  std::shared_ptr<StraightTrack> straightTrack(new StraightTrack(...));
  elementList.push_back(signalTrack);
  elementList.psuh_back(sraightTrack);
  for (Auto& currentElement : elementList){
     switch(aPropertyOfAllElements){
        case 1:
          doSomething();
          break;
        case 2:
          if (aspect==1)
             doSomething();
          else
             doSoemthingElse();
          break;
          }
      }



}

I am aware of that I can make virtual functions and overide them. But with my situation i feel that the base class should not need to know about aspect. So therefore I'm not sure if I should include the field 'aspect' in the base class element

[Repeater]

I feel that a better way to do this wouldn't be to examine each object being pointed to and thence call some function; every object should be asked to do something, and those that don't have an aspect value will execute some base functionality and those that do will execute some other, overridden functionality.

But if we know more about these functions you're calling and how they relate to the objects, maybe we'll have a better answer.

As explained above, your objects have no purpose other than to hold a couple of properties.

[TheToaster]

You would probably need to dynamic_cast the current element in the loop into a SignalTrack*. If the dynamic_cast succeeds, then you can access the method. If it is nullptr, you are not holding a SignalTrack, but the most derived type is something else, so skip it.

[jkwok678]

Every object will be asked to do something, but it's just that if it's a SignalTrack object, the doSomething() function needs to take in account of the aspect field. The doSomething is currently a place holder for qt painter. That's what I have now, but some things abstracted away. I do agree that my objects are really just keeping information. The other idea I have is using an idea from Java, which is trying to make an 'isInstanceOf' but I don't think this exists here and I'm not sure if this will work in my case.

  std::vector<Shared_ptr<Element>> elementList;
  std::shared_ptr<SignalTrack> signalTrack(new SignalTrack(...,...,...));
  std::shared_ptr<StraightTrack> straightTrack(new StraightTrack(...));
  elementList.push_back(signalTrack);
  elementList.psuh_back(sraightTrack);
  for (Auto& currentElement : elementList){
     switch(enumClassThatAllElementsHave){
        case 1:
          painter.drawImage(currentElement->getLocationX(),currentElement->getLocationY(),*straightTrackImage);
          break;
        case 2:
          if (aspect==1)
             painter.drawImage(currentElement->getLocationX(),
             currentElement->getLocationY(),*shuntTrackImage);
          else
             painter.drawImage(currentElement->getLocationX(),
             currentElement->getLocationY(),*signalTrackImage);
          else
          break;
          }
      }

}

I will have a look at static_cast, as dynamic_cast only casts upwards towards base class? Also is casting a good idea, in terms of safety?

[Repeater]

dynamic_cast will cast downwards. There is no need to cast upwards; any derived object IS a base object. The problem you have here is that you have a whole lot of base objects (or rather, pointers to base objects) and you don't know which of those base objects happen to be a particular derived object.

Casting down the chain comes with a risk and you must always check the returned pointer.

As a strong rule-of-thumb, needing dynamic_cast usually indicates that something has gone wrong in the design phase. In C++, as a rule-of-thumb, if you find that you need to program into the code an explicit discovery of what kind of object something is, something has gone wrong in the design phase; routine class specific behaviour should be part of the class - that's why we have classes!

So above, where I said this: "The problem you have here is that you have a whole lot of base objects (or rather, pointers to base objects) and you don't know which of those base objects happen to be a particular derived object." That's not quite right. The problem is that you have managed to write code in which you need to know that, and you shouldn't need to know.

How about code something like this:
painter.drawImage(currentElement->getLocationX(),currentElement->getLocationY(), currentElement->getImage());

Every element with a getImage() function; the base implementation returns one image, derived objects override that function and return a different image, or if that's wasteful they'd return an ENUM or a reference to the image to draw, or some other, but in some way the object can be asked what kind of image should be drawn. If the kind of object dictates what image should be drawn, then the object should carry that information.

This would also simplify your loop:

for (Auto& currentElement : elementList) 
{
   painter.drawImage(currentElement->getLocationX(), currentElement->getLocationY(), currentElement->getImage());
}

[TheToaster]

I agree, if you are holding a container of Element pointers, then you should only access the methods that Element provides or overrided versions of them. If you are accessing derived-specific methods, then it usually indicates bad design.

Although, if you MUST downcast, you can do so by using dynamic_cast, or in your case, since you are using smart pointers, you can use std::dynamic_pointer_cast

Don't use static_cast if you have decided to use downcasting. static_cast doesn't perform any run-time checks within a polymorphic inheritance hierarchy to see if the conversion makes sense. This could be bad if you are holding a reference to a base object but you attempt to cast it down to a derived object and proceed to call derived methods on the object.

[jkwok678]

Ok, so with what you guys have said. I guess trying to put different types of objects in the same vector is not a great idea, even when they are super-classes and sub-classes.

So in terms of design, would it be better for me to create a new class called 'map/layout' where I could have an array for each type of object? Then basically access this object each time to draw the image by iterating through all the vectors?

Also in terms of speed and memory, if I were to do this and just have a vector for each subclass of element, is there any difference?

[TheToaster]

I guess trying to put different types of objects in the same vector is not a great idea, even when they are super-classes and sub-classes.

No no no no no no no NO.

We did not say that.

You are describing polymorphism, which is a GREAT thing. However, what we are saying is that you shouldn't access specific derived class member functions that aren't in the base class Element. If you have a function that is specific to a derived class, then it is best not to call it in terms of an Element reference or pointer, because then that would require a downcast and it is usually considered bad design.

[Repeater]

Ok, so with what you guys have said. I guess trying to put different types of objects in the same vector is not a great idea, even when they are super-classes and sub-classes.

It's fine. It works really well in your situation. You have a vector of pointer-to-objects, every object needs to be drawn in the screen, they are all the same base type, this is fine.

You just have to ask each object for the thing that should be drawn. Simple, effective, makes the classes in charge of themselves, all that good stuff.

Are we missing something? What's stopping you adding a virtual getImage() function to the base class and overriding it in derived classes?


Having three different containers to hold these items is just moving the complexity; good objects answer questions about themselves. Since each object clearly represents something to be drawn on screen, then the object should indicate what is to be drawn - it should be possible to ask the object what to draw (or alternatively, just tell the object to draw itself, given that the object knows the location to draw at and the image to draw, the object is quite capable of drawing itself).

  for (Auto& currentElement : elementList)
  {
     currentElement->drawSelf();
  }

[jkwok678]

The thing with every object storing an image is that it would add to the size of each object, I could do that I think, but I would duplicate the Image object and if there are thousands of objects, it could add up unless they are all pointer. Also currently the images are all stored in the canvas class where I draw everything, but this could change I guess.

Right now most classes, are drawn based on the enum class them have, but the signal class is separate as it also depends on the field aspect. I guess maybe if I set the image earlier, at the creation of the object it could work better.

1 way I can think of changing the design is creating a new object for the different types of signalTrack/ change the enum.

The other method which I think might be the better solution is making the map object, which internally has vectors for each class, which would mean that I could retain all the specific methods and fields to all classes. I feel like this might be the better method going forward.

[Repeater]

The thing with every object storing an image is that it would add to the size of each object,

Unless they didn't store the actual duplicated image over and over. You are already creating these actual images somewhere, yes? So all each object needs to do is return a pointer or reference to that image.

You could, for example, have a function somewhere:

image* getImageFromEnum(enum which_image)
{
  if (which_image == STANDARD_IMAGE)
  {
    return a_pointer_to_the_standard_image;
  }
  else if ( ... etc etc ... )
}

Your objects would then have a base function getImage of form:

image* getImage()
{
  return getImageFromEnum(ENUM_VALUE_HARD_CODED_IN_THIS_DERIVED_TYPE)
}

where ENUM_VALUE_HARD_CODED_IN_THIS_DERIVED_TYPE is different for each derived type.

Or even just don't bother with enums and so on; if each type will always need to return the same image, you don't need enums to work out what type of object you're dealing with - have the derived class just return the correct image for that class.

Another option; if you made the image itself a static member variable of each derived class, then there would only ever be ONE instance of it in memory, shared by all the instances of the classes. The language wants to work with you; you just have to let it! This does come with time; learning C++ syntax is only the start, and knowing the syntax is not the same an knowing how to think in C++, how to work with the flow of the language.

The images wouldn't have to exist at the time the objects are created. They're only needed when you have to actually draw something on the screen.

I get the feeling you're still thinking quite low-level; you're doing for yourself various administrative things that C++ wants to do for you. Like giving each derived type an enum so you can tell what type it is; no, the compiler and the running program should know about that and should call the right function for you, without you having to manually direct things - this is what inheritance is for. If you do it all yourself, then don't bother with inheritance, since you're not making any use of what inheritance offers - just have one kind of object.

[jkwok678]

With regards to the map object with multiple vectors, what's not great about that? I would prefer to keep entire functionality of the classes.

Also with the enums I use them to specify which specific type of StraightTrack I want.
so on a 2d pane. I could have StraightHorizontal or StraightVertical and 2 more diagonal ones. I have the enum instead of having 4 different objects as direction doesn't matter in this case, but direction will matter for the signalTrack so that would require 8 different classes, and the enums, just simply the class to SignalTrack and there's 8 different directions it can go.

[Repeater]

It does sound like inheritance isn't the right option here. Sounds like you want one kind of object, with some internal variables that dictate which picture should be drawn to represent that object.

In which case, one vector of objects, all the same type, sound like what you need, with a function that takes in those member variables and from them calculates which is the correct picture.

Will those variables change over time? Or can the right image be known at the point of creation, in which case you don't need to store those variables - just a reference to the right image, or a single variable indicating the right image.