Generic method using instanceof and can it be avoided?

Question

I have the following generic interface/classes

public interface AnInterface <T>{
    T convertToY();
    String getName();
    String getLastName();
}  

public class Foo implements AnInterface<Foo>{
   // some methods specific to Foo  
}   

public class Bar implements AnInterface<Bar>{
   // some methods specific to Bar
} 

I created a generic method as follows (I didn't think it would work but it does)

public static <T> List<Person> getPersons(AnInterface<T> in) {
        System.out.println(map.get(in));
        List<Person> list = new ArrayList<>();
        if (in instanceof Foo) {
            Person p = new Person(in.getName(), in.getLastName());
            p.setId(((Foo) in).getId());
            list.add(p);
        }
        else if(in instanceof Bar) {
            Person p = new Person(in.getName(), in.getLastName());
            p.setCC(((Bar) in).getCC());
            list.add(p);
        }
        return list;
    }  

The issue is that I did not create the classes so I am not sure if doing public class Foo implements AnInterface<Foo> is something usual or not.
So what I am interested in is if there is any issue with the generic method and using instanceof and if there is any problems that can be created. E.g. if I recall correctly we can't use instanceof on collections so I was wondering if I might make it easy to introduce some other bug with this approach.

Note:. I can't modify AnInterface or Foo or Bar

Answer 1

You can turn the type specific operation into a parameter. E.g.

public static <T extends AnInterface<T>>
    List<Person> getPersons(T in, BiConsumer<T, Person> setup) {

    List<Person> list = new ArrayList<>();
    Person p = new Person(in.getName(), in.getLastName());
    setup.accept(in, p);
    list.add(p);
    return list;
}

This moves the responsibility to the caller or, if they also don’t know the actual type of T, to their callers.

Foo f = new Foo();
List<Person> persons = getPersons(f, (in, p) -> p.setId(in.getId()));

Bar b = new Bar();
List<Person> persons = getPersons(b, (in, p) -> p.setCC(in.getCC()));

You could also provide overloads, so callers knowing the exact type do not require the second parameter

public static List<Person> getPersons(Foo in) {
    return getPersons(in, (foo, p) -> p.setId(foo.getId()));
}

public static List<Person> getPersons(Bar in) {
    return getPersons(in, (bar, p) -> p.setCC(bar.getCC()));
}

But generic callers would still require the other method and have to move the responsibility for providing the operator to their callers…

---

What’s puzzling, is the existence of the convertToY() method in the interface and the original declaration of the parameter type as AnInterface<T>. Maybe, the intention behind this design was using something like

public static <T>
    List<Person> getPersons(AnInterface<T> in, BiConsumer<T, Person> setup) {

    List<Person> list = new ArrayList<>();
    Person p = new Person(in.getName(), in.getLastName());
    setup.accept(in.convertToY(), p);
    list.add(p);
    return list;
}

But for the code as given in the question, it has no advantage. The caller will look exactly the same.

Answer 2

There's a little you can do, if you're not allowed to modify these classes and interface.

Since you've taken such route that implementations of Foo and Bar diverged from another, and you can't interoperate between them, I would advise splitting this method into two independent well-readable overloaded methods:

public static List<Person> getPersons(Bar bar) {
    
    List<Person> list = new ArrayList<>();
    
    Person p = new Person(bar.getName(), bar.getLastName());
    p.setCC(bar.getCC());
    list.add(p);
    
    return list; // use List.of(p) if you don't need this list be modifiable
}

public static List<Person> getPersons(Foo foo) {
    
    List<Person> list = new ArrayList<>();
    
    Person p = new Person(foo.getName(), foo.getLastName());
    p.setId(foo.getId());
    list.add(p);
    
    return list; // use List.of(p) if you don't need this list be modifiable
}

In case if you want to keep a single method expecting an instance of the super type, these one small enhancement I can suggest.

Instead of performing casting after instanceof-check you can make use of the Pattern Matching for instanceof which was introduced with Java 16.

if (in instanceof Foo foo) {
    Person p = new Person(foo.getName(), foo.getLastName());
    p.setId(foo.getId());
    list.add(p);
}

Answer 3

public class Foo implements AnInterface<Foo>{...}

Maybe the code works, but what is the meaning of this? It's unclear, since the foo class doesn't have an implementation yet.

public class Foo implements AnInterface<T>{...}

This is not your case, but a more generic version, so still interesting. A non-generic class that extends a generic one. Seems it's considered an antipattern. See this thread

Answer 4

Just add an applyToPerson method to AnInterface. This ensures that any implementation of AnInterface considers what it needs to do in getPersons. In your implementation a new subclass of AnInterface will result in getPersons silently returning an empty list.:

import java.util.ArrayList;
import java.util.List;

interface AnInterface <T>{
    Person applyToPerson(Person p);
    String getName();
    String getLastName();
}

class Person {

    public Person(String name, String lastName) {
        
    }

    void setId(int id) {}
    void setCC(int cc) {}
}

class Foo implements AnInterface<Foo>{
    int getId() { return 0; }
    @Override
    public Person applyToPerson(Person p) {
        p.setId(getId());
        return p;
    }

    @Override
    public String getName() {
        return null;
    }

    @Override
    public String getLastName() {
        return null;
    }
}

class Bar implements AnInterface<Bar>{
    int getCC() {
        return 0;
    }
    @Override
    public Person applyToPerson(Person p) {
        p.setCC(getCC());
        return p;
    }

    @Override
    public String getName() {
        return null;
    }

    @Override
    public String getLastName() {
        return null;
    }
}

public class Application {
    public static List<Person> getPersons(AnInterface in) {
        List<Person> list = new ArrayList<>();
        list.add(in.applyToPerson(new Person(in.getName(), in.getLastName())));
        return list;
    }
}