How to iterate only over specific kind of elements?

Question

I have an array list as such:

private List<GameObject> gameObjects = new CopyOnWriteArrayList<GameObject>();

GameObject can be one of 3 classes: Spaceship, Beam and Asteroid. They all are similar so I keep them in one array. However spaceships have addition method shoot which is used every 100ms in other thread (which is called ShootRunnable). So I would like to iterate in it only over Spaceship because other doesnt implement shoot method. What is the best way to achieve this?

for (GameObject ob : gameObjects) {
  if (ob instanceof Spaceship) {
    ob.shoot();
  }
}

Can I iterate over it using something like the above? Just with the use of a cast or something? Please help.

score 2 · Answer 1 · answered Jan 20 '13 at 18:22

2

The path you're on is technically feasible, though a good rule of thumb is that if you start using reflection, you're probably doing something wrong. In this case, it might be wisest to have a two collections, one for all your game types, and one specifically for spaceships.

answered Jan 20 '13 at 18:22

Will

6,179
4
31
49

That's what I thought about too. But then every action connected with spaceship I will have to do twice. So when I add new spaceship I will have to add it both to gameObjects list and spaceships? And when remove, remove it both from gameObjects and spaceships. :/ But you think its the best way? – Michał Tabor Jan 20 '13 at 18:26
Correct. Don't worry, computers are fast. :) – Will Jan 20 '13 at 18:27
Ok so that's how I will do. Thanks a lot – Michał Tabor Jan 20 '13 at 18:27
Although instanceof is not cheap, maintaining 2 collections is crazy idea. Better to use instanceof – Archer Jan 20 '13 at 18:39
How wrong is this approach? Create your own class implementing the List interface and create another list inside that class called spList. Override the add(E e) and remove(Object o) methods of the interface to add and remove a ref to/from the spList if the object is an instanceof Spaceship. – Rafay Jan 20 '13 at 18:48
@archer What if the long iterations are costly? Would there be any better alternative besides instanceof? – Rafay Jan 20 '13 at 18:49
1

Charlie gave me the reference to this thread with benchmark results: http://stackoverflow.com/questions/103564/the-performance-impact-of-using-instanceof-in-java – Archer Jan 20 '13 at 18:51
I agree that the use of instanceof (or every other type comparison) might be a sign of something wrong in your design. However, the adoption of two different collections is an overkill. Polymorphism is the key: I would suggest to encapsulate the behavior that triggers the object action into the object itself as suggested in the other answers. – Jack Jan 21 '13 at 08:50

brabec · Answer 2 · 2013-01-20T18:50:41.103

2

In your game, are there any other actions that happen periodically?

If so, you could change the shoot() method into an abstract method (could be named periodicAction()) and place it in the GameObject class. The Spaceship class would implement this method by shooting, the other class with its specific periodic behavior and the Asteroid class by doing nothing.

edited Jan 20 '13 at 18:50

answered Jan 20 '13 at 18:37

brabec

4,632
8
37
63

adds unnecessary complexity – Archer Jan 20 '13 at 18:45
@archer: no it does not. It is a common practice on game development to add an update/step/tick/think function on every entity in the game. Take the doom 3 code of the idEntity as example (line 172): https://github.com/id-Software/DOOM-3/blob/master/neo/game/Entity.h – Jack Jan 21 '13 at 08:45

score 0 · Answer 3 · answered Jan 20 '13 at 18:27

0

You should put your shoot() method into the Spaceship class beacause no asteriod or beam will use this method. Done this, you should also keep at least two collections. One of them should only contain your spaceships.

answered Jan 20 '13 at 18:27

windler

57
4

score 0 · Answer 4 · answered Jan 20 '13 at 18:45

0

I would rather do this way:

 Iterator<Spaceship> getSpaceships(){
     // return Iterator over Spaceship objects through use of instanceof
 }

 ...
 Iterator<Spaceship> it = getSpaceships()
 while(iter.hasNext()) it.next().shoot()

To me it looks more clear.

answered Jan 20 '13 at 18:45

Archer

5,073
8
50
96

1

"Anytime you find yourself writing code of the form 'if the object is of type T1, then do something, but if it's of type T2, then do something else,' slap yourself." --Scott Meyers, Effective C++ – brabec Jan 20 '13 at 18:54
that's not mine design man ;) I'm just trying to suggest a feasible soution – Archer Jan 20 '13 at 18:56
but you suggest the use of instanceof, only hidden inside an iterator – brabec Jan 20 '13 at 19:04
there's no better way. read comments on other answers and you will decide the same. instanceof actually is not that expencive and this is a `natural` way to get class type information without the need to maintain separate enum or list. I've proposed that way only to separate instanceof comparision with call to `.shoot()` which (IMHO) is better to do on objects which are already separated into another list/set/iterator – Archer Jan 20 '13 at 19:14

score 0 · Answer 5 · answered Jan 20 '13 at 19:14

You could create an interface Action or a method into GameObject:

public interface Action {
   void performAction();
}

public abstract class GameObject implements Action {
   //...
   public void performAction() {
      // subclasses should override this method.
      // otherwise, it will do nothing.
   }
}

and then, implement Spaceship:

public class Spaceship extends GameObject {
    @Override
    public void performAction() {
        this.shoot();
    }
}

To Beam and Asteroid, you can leave the method performAction:

Then, you can interate over:

for (GameObject ob : gameObjects) {
  ob.performAction();
}

How to iterate only over specific kind of elements?

5 Answers5