is it bad to store different child classes as parent class in java

user_5590757 0
Viewed 351

Is it considered bad practice to store objects as their parent class and compare object.getClass() with an expected class to cast to the sub class.

explanation: as part of a 2D game with tile based movement I have several types of "GameObject" objects like "Portal", "Player", "NPC", "Chest", etc. Each is stored in a 2 dimensional array making up each Level which also acts as an update and rendering queue and the GameObject embodies the information required by the Level. If I store all objects as the parent type GameObject then cast them to their actual class for example (in Player class):

private void checkPosition(x, y) {
    GameObject obj = Level.getObject(x,y);
    if(obj instanceof NPC) {
       NPC npc = (NPC)obj;
       //interact with npc
    } else if(obj instanceof Chest) {
       //handle chest
    }
}

Would this generally be harder to read? What problems am I likely to have doing things this way? what would be a better method if any provided each object uses the array to interact with each other and I will likely have other types of "GameObjects" in the future, each with fundamentally different functionality.

sorry if this is wordy as I am not really sure how to explain this question.

edit: I should clarify similar code may be run from other sub classes of GameObject, not just Player (ie NPC or a later Class) and the emphasis is on being able to change the interaction between each sub class without changing the storing Level class each time one of it's potentially stored class type is created/changed. thank you to those who recommended instanceof I have updated the question to focus on other parts, but I will likely be using it from now on. Thanks for the feedback.

Conclusion: I have decided to go for a variant of daniu's answer where each GameObject has its own AI subclass which extends a stub/adapter of the mentioned interface. Thus I don't have to implement reactions for ever class, I can give different behaviours to the same object type and handling interaction should look like this:

getObject(x,y).getAI().react(obj);
casting java
3 Answers

This is a use case for the visitor pattern.

Add an interface (for everything that may move through the level)

interface LevelCrawler {
   void accept(Chest chest);
   void accept(NPC npc);
}

and to your GameObject

interface GameObject { // or abstract class
    // added method
    void accept(LevelCrawler lc);  // abstract if class
}

The LevelCrawler would be implemented by eg the Player class (or NPC etc as you describe in your edit).

class Player implements LevelCrawler {
   public void accept(Chest chest) { ... }
   public void accept(NPC npc) { ... }
}

containing the code in the if () {...} blocks.

Then, you have eg

class Chest implements GameObject {  // I think it's a class, so probably "extends"
    void accept(LevelCrawler p) { p.handle(this); } // to call Player#handle(Chest)
}

and your code looks like

GameObject o = Level.getObject(x, y);
o.accept(player); // or NPC or whatever is moving right now

Look, it works.

The advantage of this is that when you create a new GameObject class, you can easily add the new behavior to the existing LevelCrawler classes and leave the checkPosition code unchanged.

Edited version with both Player and NPC moving through the dungeon.

user_28377410

I'd recommend using instanceof instead of comparing the class itself.

private void checkPosition(x, y) {
    GameObject obj = Level.getObject(x,y);
    if(obj instanceof NPC) {
       NPC npc = (NPC)obj;
       //interact with npc
    } else if(obj instanceof Chest) {
       //handle chest
    }
}

instanceof is an operator for testing that an object is of a specified type i.e. class, subclass or interface.

Have a read of the tutorial found here for extra help.

Before casting an unknown object, the instanceof check should always be used. Doing this helps in avoiding ClassCastException at runtime.

user_9800970

Instead of 

if(obj.getClass() == NPC.class)

it would be better to use

if (obj instanceof NPC)

unless you want to test the exact class without allowing subclasses.

Another option could be to let the objects check their positions on their own and just do

private void checkPosition(int x, int y) {
    GameObject obj = Level.getObject(x,y);
    obj.checkPosition()
}

or 

private void checkPosition(int x, int y) {
    GameObject obj = Level.getObject(x,y);
    obj.checkPosition(this)
}

depending on what is needed for this position check.

user_2969740
Related