How to avoid casting from superclass?

Viewed 135

I am developing a game with different game entities. It works quiet well, but i would like to get rid of some casting operations in my code. For example, when i am checking if a Bullet hits an Enemy, i need to cast both objects to able to reduce the health (property of Enemy) based on the damage (property of Bullet).

I store the entities in a map together with their corresponding classes. It looks like this:

Map<Class<? extends Entity>, List<Entity>> entities;

Here are my methods to put, receive and remove entities from the map:

void add(Entity entity) {
    Class<? extends Entity> type = entity.getClass();
            
    if (!entities.containsKey(type)) {
        entities.put(type, new CopyOnWriteArrayList<>());
    }
    
    entities.get(type).add(entity);
}

List<Entity> getAll(Class<? extends Entity> type) {
    return entities.getOrDefault(type, Collections.emptyList());
}

void remove(Entity entity) {
    getAll(entity.getClass()).remove(entity);
}

Finally here is my code (which runs in a game loop) to check if a Bullet hits an Enemy:

for (Entity bullet : data.getAll(Bullet.class)) {
        for (Entity enemy : data.getAll(Enemy.class)) {
            if (bullet.box.overlaps(enemy.box)) {
                // Bullet hits Enemy
                Bullet bulletHit = (Bullet) bullet;
                Enemy enemyHit = (Enemy) enemy;
                
                enemyHit.health -= bulletHit.damage;
                if (enemyHit.health <= 0) {
                    data.remove(enemyHit);
                }
                data.remove(bulletHit);
                
                break;
            }
        }
    }

Is there any way to avoid these casting operations for the Bullet and the Enemy? One solution i was thinking about, is to get rid of the the map and just use many list of those specific entity types, but this would infalte my code.

2 Answers

„…Is there any way to avoid these casting operations for the Bullet and the Enemy?…“

TL;DR: Yes. There is. Consider this simple approach.



The long answer

Here's one way to do it (the simplest way, in my opinion)…

...
static void play(Gamer data){ 
     for ( Entity bullet : data.getAll(Bullet.class)) {
        for (Entity enemy : data.getAll(Enemy.class)) {
            if (bullet.getBox( ).overlaps( enemy.getBox( ) ) ) {
                // Bullet hits Enemy
                Damagable bulletHit = bullet;
                Illable enemyHit = enemy;
            
                int health = enemyHit.getHealth( );
                int damage = bulletHit.getDamage( );
                enemyHit.setHealth( health -= damage  );
                if ( health <= 0 ) {
                    data.remove(enemy);
                }
                data.remove(bullet);
            
                break;
            }
        }
    }       
}
...

This way involves the introduction of the two intefaces: Damagable and Illable (of course, you can rename those to anything that expresses your intent more precisely).

Like every design choice, there are trade-offs. But this is a simple way to get rid of those casts.

Click the green Start at the top of the page in that link to run the experiment



The following simplifying refactors emerged from the iterative process that I like to call „Experiment-driven Development“ (EDD)…

...
/*Damagable bulletHit = bullet;
Illable enemyHit = enemy;*/
            
int health = enemy/*Hit*/.getHealth();
int damage = bullet/*Hit*/.getDamage();
enemy/*Hit*/.setHealth(health -= damage);
...

The EDD process includes RPP („Remote Pair Programming“). From the insightful observations that @DavidL made while I „drove“ (see comments) he realized that a side effect of the simpler proposed solution makes his code even simpler than what his original question requested.

The original question being: „How to avoid casting from superclass?“ .

This proposed solution is so flexible that the OP spotted an opportunity to reduce his original code by 2 whole lines plus 9 unnecessary characters (the commented out stuff above).

„…But then i would have to implement those interfaces for ALL entities…“

In the proposed solution, you would have to implement Entity. You currently do that anyway. You'd have to implement it regardless of which design you go with.

Automatically-generated starter methods for any additional implementations would take no more effort than a single mouse click in a modern IDE. Unless you're writing your game in your OS's equivalent of TextEdit?

In your current code, you have to implement whatever behavior Bullet requires and whatever behavior Enemy calls for. Regardless of which design you opt for, you'd still have to implement those same behaviors.

If we can guarantee that we only store non-generic objects within entities, then we can make the accessors type-safe (although we have to cast on one occasions but we will justify this cast). The concept we are going to use is very similar to what ArrayList does, using an Object[] as internal data structure.

Warning: the implementation groups objects by their class-type. Interface-types are completely ignored.

We are going to treat entities as internal data structure, i.e. we cannot allow to leak any references to entites to the outside. With this parameter set we introduce an internal method that gets a List from the map, casted to the correct type:

@SuppressWarnings("unchecked")
private <T extends Entity> List<T> getListCasted(Class<? extends T> type) {
    return (List<T>) entities.getOrDefault(type, Collections.emptyList());
}

Notice that this list is set private, i.e. only for class-internal usage.

Now, we rewrite getAll(...) and remove(...) to use this method:

private <T extends Entity> List<T> getAll(Class<? extends T> type) {
    return new ArrayList<>(getListCasted(type));
}

public void remove(Entity entity) {
    getListCasted(entity.getClass()).remove(entity);
}

Notice that getAll(...) now returns a (mutable) copy of the internal List.

We do not need to modify method add(...).

Ideone Demo

Now we need to justify the unchecked cast we made within getListCasted(...). If we take a look at method add(...), we see that the class-type (key) is the type of the containee of the list. Thus, we can guarantee that under the key of some Class<T extends Entity> a List<T> is stored. Therefore, the cast is justified.

We can even forego the bounding on the Map, using a Map<Class<?>, List<?>>.

Ideone Demo

Related