Multiple lists or list and getSublist() (Java)

Question

I have an abstract class 'entity' and Objects (that extend 'entity') that implement different interfaces. I also have an ArrayList that contain all those different Objects.

Now if I need access to all entities that implement a certain interface (to use its methods), I use the following method (which returns a filtered list of 'entities' that implement interface 'IDirectFire'):

public ArrayList<IDirectFire> getDirectFireSublist() {//direct fire sublist
    ArrayList<IDirectFire> sublist = new ArrayList();
    entities.stream().filter((it) -> (it instanceof IDirectFire)).forEach((it) -> {
        sublist.add((IDirectFire) it);
    });
    return sublist;
}

Now to my question: Should I further work with this method or should I create a new ArrayList that exists besides 'entities' and that I would need to manually update every time 'entities' changes?

I need to update 'entities' a lot so I'm not sure if it's more efficient to store multiple sublists and update them all every time 'entities' changes or if I should keep using methods to filter 'entities' and apply methods to those sublists. Keep in mind that those sublists would also be used in a loop in other methods e.g.:

private void resetFirestatusIDF() {//reset firestatus (IDirectFire)
    getDirectFireSublist().stream().forEach((it) -> {
        it.dfHasFired(false);
    });}

Is this viable? Thanks in advance!

In general one should try to design to avoid the use of `instanceof`, where possible, as it is considered a "design smell" — wakjah, Jul 17 '16 at 13:26

davidxxx · Answer 1 · 2016-07-17T14:01:13.067

1

Now to my question: Should I further work with this method or should I create a new ArrayList that exists besides 'entities' and that I would need to manually update every time 'entities' changes?

For which reason do you want duplicate the 'entites' data ?

1) You can put them only in a dedicated list. In this case, you don't need getDirectFireSublist() anylonger.

2) You can share them between the two lists without duplicating them. In this case, you must update the added and the removed entity element because only the modified elements will be updated. But it is rather straight to implement.

edited Jul 17 '16 at 14:01

answered Jul 17 '16 at 13:02

davidxxx

125,838
23
214
215

The new list would not be a duplicate but the filtered 'entities' as its own list, so yes I meant a dedicated list. Thanks! – Jul 17 '16 at 13:22

score 0 · Answer 2 · answered Jul 17 '16 at 13:01

It is better to filter them. It will create a more clear and understandable code at a price of negligible performance decrease which unless you are filtering milliards of elements should be negligible.

The second thing I have noticed is you stream usage for code fragment 1. I would recommend you and alternative approach to it :

> public ArrayList<IDirectFire> getDirectFireSublist() {
>      return entities.stream().filter((it) -> (it instanceof IDirectFire)).collect(Collectors.toList());
> }

Thanks a lot! I wasn't sure how much of an impact it would have on performance. — , Jul 17 '16 at 13:22

score 0 · Accepted Answer · answered Jul 17 '16 at 13:08

0

If all you need is to loop over a subset of your items, creating a new list is wasteful. Just return the filtered Stream.

public Stream<IDirectFire> getDirectFire() {
    return entities.stream().filter((it) -> (it instanceof IDirectFire));
}

You could also use Guava and return a filtered Iterable instead:

public Iterable<IDirectFire> getDirectFire() {
    return FluentIterable.from(entities).filter(IDirectFire.class);
}

Then, to loop over the items elswhere:

private void resetFirestatusIDF() {
    getDirectFire().forEach((it) -> it.dfHasFired(false));
}

answered Jul 17 '16 at 13:08

Sam

8,330
2
26
51

Thank you, although I'm currently not using Guava. Will take a look at it! – Jul 17 '16 at 13:23
The `filter(Class>)` method in Guava is neat for what you're doing, but other than that the `Stream` method is just as good. They both have a `forEach` method, which means the looping example is identical for both. – Sam Jul 17 '16 at 13:30

Sam · Answer 4 · 2016-07-17T13:53:46.520

wakjah mentions in a comment that instanceof is a bit of a design smell. With that in mind, one alternative solution would be to use a Visitor pattern.

public abstract class Entity {
    public abstract void acceptVisitor(EntityVisitor visitor);
    ...
}

public interface IDirectFire {
    default acceptVisitor(EntityVisitor visitor) {
        visitor.visit(this);
    }
    ...
}

public class ResetFireStatusVisitor implements EntityVisitor {
    public void visit(IDirectFire directFireEntity) {
        directFireEntity.dfHasFired(false);
    }
}

Then, to loop over the items:

entities.forEach(entity -> entity.acceptVisitor(new ResetFireStatusVisitor()));

The ResetFireStatusVisitor calls dfHasFired(false) on anything that implements IDirectFire. In EntityVisitor you can specify default no-op implementations for the other subtypes of Entity.

I'm not suggesting you do this for simple cases, but for large-scale designs this might be a useful answer to this problem. On the other hand, it might not -- this pattern has its share of design smells too.

May I ask why instanceof is design smell? I definitely see the usefulness of the visitor, just curious. Is instanceof more prone to errors? Thanks a lot! — , Jul 17 '16 at 14:00
Too big of a subject area to cover in a comment, but there's plenty of discussions out there. Here's the top search result: http://stackoverflow.com/questions/20589590/why-not-use-instanceof-operator-in-oop-design — Sam, Jul 17 '16 at 14:02
Yeah sorry, obviously a big subject, I'll do some searching. Thanks for the link and your help! — , Jul 17 '16 at 14:07

Multiple lists or list and getSublist() (Java)

4 Answers4