4

I'm coding a game and I have a design issue in which I have a superclass which all items in the game inherit from. The game is grid based, and the items can be placed at different parts of the grid. The grid has a dictionary of grid positions -> BaseItem.

The problem is, the derived items all do very different things. So I always have to downcast to get the specific behaviors I want when I retrieve stuff from the grid. Here's a simple example:

public abstract class BaseObj {
  public string Name {get; set;}
  public int Hitpoints {get; set;}
}

public class Turret : BaseObj {
    public void FireWeapon() { }
    public int Damage {get; set;}
}

public class ProductionBuilding: BaseObj {
    public void CraftItem(BaseObj item) {}
    public float CraftSpeed {get; set;}
}

So a scenario is where the player is selecting an item from the grid. The grid class has a method that returns the item located at the coordinates of the mouse when clicked.

From here, I need to determine what kind of thing the player has selected. It will determine which UI panels I show and what behaviors I can do on the items. As a result my code base is littered with downcasts and typechecks. There must be some design pattern that deals with moderately complex inheritance structures and downcasting. Any ideas?

I'm coding in C# and Unity, but I imagine the answer will be general enough to encompass OOP generally.

This is not a duplicate of :How to avoid the continuous downcasting in this case?

In that case, the container object only housed one Dog, so applying a template type to DogHouse was easy enough. But this solution cannot be applied to my grid, as it contains a list of items.

EDIT:

As requested, here's an example of where I'm casting. The context of this is the player is placing down an item on the grid.

  if (typeof(StationModule) == item.GetType())
  {
    StationManager.HandleModuleAdded((StationModule)item);
  }

The StationManager does some special stuff when StationModules are placed, but the player can place other things besides StationModules onto the grid, in which case StationManager does not care.

5
  • If they objects have nothing in common, and you need to downcast them everytime, why not just store them by the derived type? And what's the intention of the base class here?
    – Bernhard
    Dec 1, 2018 at 19:42
  • My thinking was that the grid needs to be able to store any kind of item in the game, but it doesn't really need to know the specifics of what its containing. Also there is stuff in common that isn't listed in the simple example, like Name, Hitpoints, Id, Size, etc.
    – Slims
    Dec 1, 2018 at 19:59
  • 2
    Possible duplicate of How to avoid the continuous downcasting in this case?
    – gnasher729
    Dec 1, 2018 at 20:17
  • It is not, see edit. I need the Grid container to contain a list of BaseObj, so I cannot simply apply a template. The list contains all different subtypes of BaseObj.
    – Slims
    Dec 1, 2018 at 20:21
  • 1
    Use the tell don't ask idiom pragprog.com/articles/tell-dont-ask Dec 2, 2018 at 12:55

4 Answers 4

10

From here, I need to determine what kind of thing the player has selected. It will determine which UI panels I show and what behaviors I can do on the items. As a result my code base is littered with downcasts and typechecks. There must be some design pattern that deals with moderately complex inheritance structures and downcasting. Any ideas?

The design pattern is called "put that stuff in the class where it belongs". If the arrangement of UI panels and "behaviors I can do on the items" are defined based on the object's dynamic type, then the interface functions for performing that task ought to be in the base class.

That is, your BaseObj type should have a "SetupPanels" virtual function that the derived types specialize. Exactly how that works is up to you (it could actually create the panels, or return a data structure that describes which panels to create, or whatever), but it should be a fundamental part of the interface of BaseObj, not something that lives outside of BaseObj.

Basically, most of the times when you start "downcasting" (or even conceiving of that word), then one of two things is going on: you either have not added the right functionality to the common base class, or you shouldn't be using inheritance to begin with.

The StationManager does some special stuff when StationModules are placed, but the player can place other things besides StationModules onto the grid, in which case StationManager does not care.

If "placing" an item needs to let someone know that's happening based on what type it is, then your base class should have a virtual member that gives the item the chance to tell whomever it is that needs to know. It's a concept that is integral to the fact that it is a StationModule, so StationModule ought to know about it.

4
  • 1
    I've been combing through my code base looking at each instance of my casts, and I believe you're more or less correct here. The problem is that my code is sort of fundamentally designed improperly, at least in this regard. I will have to go through and refactor and redesign as necessary (which given how large my code base is might take some time). Thanks for helping me think about this problem.
    – Slims
    Dec 1, 2018 at 21:34
  • 2
    I think this answer is not bad, but only half of the story. A SetupPanels method does not belong into BaseObj when such an object should be kept UI agnostic. Instead, any derivation of BaseObj could provide enough meta information to allow a SetupPanels method´ in the UI layer to be implemented in a generic fashion.
    – Doc Brown
    Dec 2, 2018 at 12:49
  • @DocBrown: Hence my saying "it could actually create the panels, or return a data structure that describes which panels to create" The nature of that data structure is dealer's choice. Indeed, it doesn't even have to be about "panels" specifically. If those panels are for the user to select a set of actions for the unit, then the function would return the list of available actions for the unit. And probably not be named SetupPanels. Dec 2, 2018 at 14:18
  • 1
    +1 "for or you shouldn't be using inheritance to begin with" - too often the only tool an OP programmer uses is the inheritance hammer, so every problem is viewed as a nail.
    – mattnz
    Dec 5, 2018 at 4:15
2

This is a common problem when using generic container like a grid to store different subtypes of an object. The first approach then is surely to check if the base class contains the correct methods so it allows to solve the issues by polymorphism directly.

However, often the required operations on the different types of objects cannot be easily implemented as polymorphic functions, because

  • they are operations in a completely different kind of layer (like the UI). The SetupPanels example in Nicol Bolas' answer could be such a method, so the solution is often not so straightforward as that answer imposes.

  • they would put responsibilities into the objects which clearly don't belong there.

A possible solution to this is to make your subtypes provide meta information about themselves, so the operations can be implemented more generically. The idea is that a layer like the UI could ask the object about its capabilities, like which displayable properties it has, which types these properties have etc. This could be implemented either in terms of polymorphic functions telling your program about the available functions/features, or (in C#) by making use of attributes. Of course, to collect those information you need to make use of reflection.

Another approach could be to make your base object provide different injectable strategy objects. For example, the base object could provide a property DrawingStrategy, which is initialized once during object construction. The abstract interface IDrawingStrategy then could have different subtypes from the UI layer. That goes a step further than just giving your base object a polymorphic Draw method.

During object initialization, the program then needs to inject the correct strategies into the correct subtypes. But that is acceptable, since "object creation" is the place in your program where the different subtypes need to be known. The idea is, to keep that the one-and-only place, so your code base does not get cluttered with type checks in arbitrary places.

2

Invert the call direction

The most common way to avoid downcasting is to invert the direction of the call. The grid does not know what type of BaseObj is contained in a tile, but the BaseObj knows exactly what kind of grid it is contained in, right?

Two ways to invert the direction come to mind.

Event based

  1. Create a Tile class that wraps a BaseObj.

    class Tile
    {
        event ClickEventHandler OnClick;
    
        protected readonly BaseObj _gameObject;
    
        public Tile(BaseObj gameObject)
        {
            _gameObject = gameObject;
        }
    
        public void Click()
        {
            this.OnClick?.Invoke(this, new EventArgs());
        }
    
        public BaseObj GameObject => _gameObject;
    }
    
  2. When adding the game object to the grid, wrap it in a new Tile and subscribe to its events. Since you just instantiated the game object, you know what type it is and how you're supposed to subscribe to it.

    var turret = new Turret();
    var tile = new Tile(turret);
    tile.OnClick += (s,e) => turret.FireWeapon();
    grid[x,y] = tile;
    
  3. When the user clicks the tile, handle that click by calling the tile, not the turret.

    grid[x,y].Click();
    

    In response, the Tile will emit the event and it'll get sent to your handler which will fire the weapon.

  4. You can still access the original object through the GameObject property:

    var itemName = grid[x,y].GameObject.Name;
    

Simple

You can just add a Click() method to the base type and override it in each derived type. This has poorer separation of concerns but is a very simple design and requires you to add only one method to the base class.

abstract class BaseObj
{
    public abstract void Click();
}

class Turret : BaseObj
{
    public override void Click()
    {
        this.FireWeapon();
    }
}

Then when a user clicks it, just call the Click() method. Since this is defined in the base type, no casting is necessary.

One issue that may come up is that you can't pass any type-specific arguments to Click. For example, maybe the turret needs a single argument that indicates rate of fire, but no other game object needs it, so it doesn't make any sense to put it in the base class' Click method. This is where the inverted call direction comes in; the game object can still pull the information that it needs from the grid or other context.

abstract class BaseObj
{
    public abstract void Click(GameContext context);  //Common to all game objects
}

class Turret : BaseObj
{
    public override void Click(GameContext context)
    {
        this.FireWeapon(context.RateOfFire);  //Pulls only what it needs from the context
    }
}
1

Lets consider a familiar domain.

We are showing a window with two parts. First one displays a user interface layout. For simplicity lets assume there are only three UI elements allowed, TextBox, Button and CheckBox each with its own properties. They all get inherited from Element. All of them will have bounds, so bounds is placed in Element.

class Element {
    Rect bounds;
}

class TextBox extends Element {
    String hint;
}

class Button extends Element {
    String label;
}

class CheckBox extends Element {
    boolean checked;
}

When a UI element is selected, its corresponding properties should be shown in the second part of the window. For example if a Button is selected its bounds and label should be shown in the second part.

The first attempt would be to write a class PropertiesPanel as below.

class PropertiesPanel {

    void displayProperties(Element e) {

        if (e instanceof TextBox)
            displayTextBoxProperties((TextBox) e);
        else if (e instanceof Button)
            displayButtonProperties((Button) e);
        else if (e instanceof CheckBox)
            displayCheckBoxProperties((CheckBox) e);

    }
}

Assume the methods displayTextBoxProperties(), etc know how to display an individual UI element.

This solution suffers from the excessive casting problem. We can do it better. Rewrite the Element hierarchy as below.

abstract class Element {
    Rect bounds;
    abstract void display(PropertiesPanel panel);
}

class TextBox extends Element {
    String hint;

    void display(PropertiesPanel panel) {
        panel.displayTextBoxProperties(this);
    }
}

class Button extends Element {
    String label;

    void display(PropertiesPanel panel) {
        panel.displayButtonProperties(this);
    }
}

class CheckBox extends Element {
    boolean checked;

    void display(PropertiesPanel panel) {
        panel.displayCheckBoxProperties(this);
    }
}

And don't forget to remove the lengthy displayProperties() method from PropertiesPanel. Finally in the on selection event of the element write as below.

PropertiesPanel mPropertiesPanel;

void onElementSelected(Element e) {
    e.display(mPropertiesPanel);
}

I hope this solution can be translated to your domain easily.

This pattern's good name is Visitor Pattern.

Note: Here the PropertiesPanel is the visitor. And the visitor is concrete and not hierarchialized. As described in the pattern you can make a hierarchy of visitors if needed.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.