10

I've tried to do some stuff with generics already but it seems I cannot personally find any simple solution. Still I think it'd be a sin to leave these 3 similar methods alone as they are.

    public List<PassengerPlane> getPassengerPlanes() {
        List<PassengerPlane> passengerPlanes = new ArrayList<>();
        for (Plane plane : planes) {
            if (plane instanceof PassengerPlane) {
                passengerPlanes.add((PassengerPlane) plane);
            }
        }
        return passengerPlanes;
    }

    public List<MilitaryPlane> getMilitaryPlanes() {
        List<MilitaryPlane> militaryPlanes = new ArrayList<>();
        for (Plane plane : planes) {
            if (plane instanceof MilitaryPlane) {
                militaryPlanes.add((MilitaryPlane) plane);
            }
        }
        return militaryPlanes;
    }

    public List<ExperimentalPlane> getExperimentalPlanes() {
        List<ExperimentalPlane> experimentalPlanes = new ArrayList<>();
        for (Plane plane : planes) {
            if (plane instanceof ExperimentalPlane) {
                experimentalPlanes.add((ExperimentalPlane) plane);
            }
        }
        return experimentalPlanes;
    }
in7hesky
  • 111
  • 6
  • 3
    What about introducing a `PlaneType` enum that is part of the `Plane` abstraction. It makes filtering as easy as comparing for the required plane type. This of course still requires to cast the filtered instances. – Glains Jul 28 '20 at 19:05
  • The question would be how your "planes" come to be. Can you perhaps already sort them into a map or different lists when you create the "planes" collection? – Frank Hopkins Jul 28 '20 at 19:14
  • You should edit your Question to contain "generic" in the title. It will make it easier for others with a similar problem to find it. – Scratte Jul 28 '20 at 19:26
  • @Glains Yes I agree, I thought about that but it felt like there was another way – in7hesky Jul 28 '20 at 20:36
  • Which specific problem (or problems) did you have in using generics here? If `instanceof` is the only problem, this might be a duplicate of [Java: Instanceof and Generics](https://stackoverflow.com/questions/1570073/java-instanceof-and-generics) – Bernhard Barker Jul 29 '20 at 07:56

8 Answers8

8

What do you need is generic method, but the problem is that instanceof cannot check against type parameter (it is in fact erased during compilation), it requires actual class reference. So, you may provide this to the method explicitly:

public <T extends Plane> List<T> getPlanes(Class<T> claz) {
  List<T> result = new ArrayList<>();
  for (Plane plane : planes) {
    if (claz.isInstance(plane)) {
      result.add(claz.cast(plane));
    }
  }
  return result;
}

Note how instanceof and explicit cast changed to calls to .isInstance() and .cast()

Use it like

getPlanes(PassengerPlane.class)
Vasily Liaskovsky
  • 2,248
  • 1
  • 17
  • 32
  • Reflection is certainly a way to get around explicit casting :) – ajc2000 Jul 28 '20 at 19:18
  • I'm just curious, is there a reason you avoided explicit casting? Would `result.add((T) plane);` not have done the same thing as `result.add(claz.cast(plane));`? I'm not trying to say there's anything wrong with claz.cast(), I'm just wondering if there's something here that's going over my head. :) – Charlie Armstrong Jul 28 '20 at 19:31
  • Eh.., just to be aligned with `isInstance()` and hey, it's a demo, why not try something different? – Vasily Liaskovsky Jul 28 '20 at 19:34
  • Okay, thank you. As long as I'm not missing something about Java lol – Charlie Armstrong Jul 28 '20 at 19:40
  • 1
    @CharlieArmstrong: an advantage of `claz.cast(plane)` is that it actually verifies the cast whereas `(T) plane` is an unchecked cast, i.e. it just tells the compiler to assume we are right, but doesn't actually verify that `plane` is of type `T`. – Joachim Sauer Jul 29 '20 at 08:20
  • @JoachimSauer We've already verified the cast, though. I can't think of a reason for checking again to be necessary. – HTNW Jul 29 '20 at 09:20
  • @HTNW: it gets rid of the "unchecked cast" warning for one, and there's a non-zero chance that the JIT will actually optimize that away since it can see the instanceof check already happened. – Joachim Sauer Jul 29 '20 at 10:48
2

You can make things a bit shorter with Streams, but I'm not sure there's a way to get around using instanceof here:

public List<PassengerPlane> getPassengerPlanes() {
    return planes.stream().filter(t -> t instanceof PassengerPlane)
                 .map(t -> (PassengerPlane) t).collect(Collectors.toList());
}
public List<MilitaryPlane> getMilitaryPlanes() {
    return planes.stream().filter(t -> t instanceof MilitaryPlane)
                 .map(t -> (MilitaryPlane) t).collect(Collectors.toList());
}
public List<ExperimentalPlane> getExperimentalPlanes() {
    return planes.stream().filter(t -> t instanceof ExperimentalPlane)
                 .map(t -> (ExperimentalPlane) t).collect(Collectors.toList());
}
ajc2000
  • 651
  • 5
  • 20
  • 2
    `return planes.stream().filter(cls::isInstance).map(cls::cast).collect(Collectors.toList())` should do the trick (with a `Class cls` parameter). – Konrad Rudolph Jul 29 '20 at 08:55
2

Here's how I would approach the problem using generics:

public <T> List<T> getTPlanes(Class<T> clazz) { //declare the method to take a type generic
    List<T> tPlanes = new ArrayList<>(); //initialize an ArrayList of that type

    planes.stream() //stream the planes list
            .filter(clazz::isInstance) //filter it down to only planes of the type that we want
            .forEach((p) -> tPlanes.add((T) p)); //add each plane left in the stream to our new ArrayList, and cast it to the type generic

    return tPlanes; //return the ArrayList we just created and populated
}
Charlie Armstrong
  • 2,332
  • 3
  • 13
  • 25
1

You do need to do a cast somewhere: Here is a solution with a single method that takes the subtype.

import java.util.*;
import java.util.stream.*;

public class Example {
    public static class Plane { }
    public static class PassengerPlane extends Plane { }
    public static class MilitaryPlane extends Plane { }
    public static class ExperimentalPlane extends Plane { }

    private static List<Plane> planes =
        List.of(new PassengerPlane(),
                new MilitaryPlane(),
                new ExperimentalPlane());

    public static <T extends Plane> List<T> getPlanesOfType(Class<T> type, List<Plane> planes) {
        List<T> list =
            planes.stream()
            .filter(t -> type.isAssignableFrom(t.getClass()))
            .map(t -> type.cast(t))
            .collect(Collectors.toList());

        return list;
    }

    public static void main(String[] args) throws Exception {
        System.out.println(getPlanesOfType(PassengerPlane.class, planes));
        System.out.println(getPlanesOfType(MilitaryPlane.class, planes));
        System.out.println(getPlanesOfType(ExperimentalPlane.class, planes));
        System.out.println(getPlanesOfType(Plane.class, planes));
    }
}
[Example$PassengerPlane@7b227d8d]                                                                                                                                                                                                      
[Example$MilitaryPlane@7219ec67]                                                                                                                                                                                                       
[Example$ExperimentalPlane@45018215]                                                                                                                                                                                                   
[Example$PassengerPlane@7b227d8d, Example$MilitaryPlane@7219ec67, Example$ExperimentalPlane@45018215]

You could either use the single method to replace all three or use it to implement.

Allen D. Ball
  • 1,916
  • 1
  • 9
  • 16
1

It can be done in this way by introduced a method that do common part:

private static <T> List<T> createFilteredList(List<Plane> inList, Class<T> clazz) {
    List<T> outList = new ArrayList<>();
    for (Plane value : inList) {
        if (clazz.isInstance(value)) {
            outList.add(clazz.cast(value));
        }
    }
    return outList;
}

Then it can be used like this:

public List<PassengerPlane> getPassengerPlanes() {
    return createFilteredList(planes, PassengerPlane.class);
}
public List<MilitaryPlane> getPassengerPlanes() {
    return createFilteredList(planes, MilitaryPlane.class);
}
public List<ExperimentalPlane> getPassengerPlanes() {
    return createFilteredList(planes, ExperimentalPlane.class);
}
lczapski
  • 4,026
  • 3
  • 16
  • 32
1

If your problem is really so short, probably it won't be worthy the effort. However, this is a typical problem for Visitor Pattern (especially if your duplicate code is larger).

Step 1

Create a Visitor interface to visit each type of Plane:

interface Visitor {
    void visit(MilitaryPlane militaryPlane);
    void visit(ExperimentalPlane experimentalPlane);
    void visit(PassengerPlane passengerPlane);
}

... and implement it in a way that starts with a List<Plane> that can be enriched by each of the .visit():

class PlaneVisitor implements Visitor {

    private final List<Plane> planes;

    PlaneVisitor(List<Plane> planes) {
        this.planes = requireNonNull(planes);
    }

    @Override
    public void visit(MilitaryPlane militaryPlane) {
        planes.add(militaryPlane);
    }

    @Override
    public void visit(ExperimentalPlane experimentalPlane) {
        planes.add(experimentalPlane);
    }

    @Override
    public void visit(PassengerPlane passengerPlane) {
        planes.add(passengerPlane);
    }

    public List<Plane> getPlanes() {
        return planes;
    }
}

Step 2 - Enable visitors in your classes

Add an abstract method in your base class Plane to accept the visitor:

public abstract class Plane {
    //...
    abstract void accept(Visitor visitor);
    //...
}

Then implement this method in each sub-class to let the Visitor instance visit itself (this). Example for PassengerPlane (same logic for all the other classes):

public class PassengerPlane extends Plane {
    //...
    @Override
    void accept(Visitor visitor) {
        visitor.visit(this);
    }
    //...
}

Step 3 - Adapt your function

Your function can now loop through the list of planes not caring about the type. It will be resolved by the visitor:

public List<Plane> getPlanes() {
    PlaneVisitor planeVisitor = new PlaneVisitor(new ArrayList<>());
    for (Plane plane : planes) {
        plane.accept(planeVisitor);
    }
    return planeVisitor.getPlanes();
}

Note that you need to add a bit of methods / interfaces to do this. Since your code is very small, you can imagine to leave it like it is even if it's not very "elegant". However, the above example can be of inspiration if your code is actually supposed to do more than what you're showing us.

Matteo NNZ
  • 11,930
  • 12
  • 52
  • 89
1

So, you have an iterable of Plane as an input. A Plane can be PassengerPlane, MilitaryPlane or ExperimentalPlane.

What you are trying to do is to filter a collection of planes by a predicate. A predicate is a function that takes a Plane and answers true or false. A filter uses a predicate to figure out whether to include a given item into the result or to skip.

If you are using Java 8 or later version, you can use the Streams API. https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html

  1. Produce a stream from the planes iterable.
  2. Apply filter to it (intermediate operation).
  3. Collect the results to list.

Using Stream API you can convert the methods above into one-liners. Like this:

planes.stream().filter(plane -> plane instanceof MilitaryPlane).collect(toList());

Then, probably, you won't need a separate neat method for it. But if you want some reusable piece of code, then you have to figure out what is the parameter here. In the code above it is a specific plane implementation:

public List<Plane> filterPlanes(Iterable<Plane> planes, Class<? extends Plane> planeImplementation)

So, you can build a predicate with this parameter:

plane -> planeImplementation.isInstance(plane)

iTollu
  • 999
  • 13
  • 20
0

If you have a Plane supertype, you can make the subclasses inherit the getPlanes() method. this.getClass will extract only the planes of the subclass calling the method from the list. This way, you don't have to pass a class to the method, as it can be derived from the subclass that is calling it.

public abstract class Plane {

    public Plane(){}

    public List<Plane> getPlanes() {
        List<Plane> result = new ArrayList<>();
        for (Plane plane : planes) {
            if (this.getClass().isInstance(plane)) {
                result.add(this.getClass().cast(plane));
            }
        }
        return result;
    }
}

class PassengerPlane extends Plane {
}
class MilitaryPlane extends Plane {
}
class ExperimentalPlane extends Plane {
}
public class PlaneList {
    public String name;
    public static ArrayList<Plane> planes = new ArrayList<>();

    public PlaneList(){
        planes.add(new MilitaryPlane());
        planes.add(new MilitaryPlane());
        planes.add(new MilitaryPlane());
        planes.add(new PassengerPlane());
        planes.add(new PassengerPlane());
        planes.add(new ExperimentalPlane());
    }
}

I tested it like so:

public class Main {

    public static void main(String[] args) {
        PlaneList list = new PlaneList();
        Plane plane = new PassengerPlane();

        for(Plane p : plane.getPlanes()){
            System.out.println(p.toString());
        }
    }
}

output:

com.company.PassengerPlane@7dc36524
com.company.PassengerPlane@35bbe5e8
Epic Martijn
  • 335
  • 2
  • 12