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

`Foo` (as well as `Bar`) is declared as subtype of `AnInterface`, so that the compiler is fine with the downcast, and using the `instanceof` operator makes perfect sense. The only problem is, that your method is hardcoded to those two types. Usually it's better to make use of polymorphism like user tgdavis did in his answer. — Mihe, Oct 22 '22 at 22:10
@Mihe: I guess the alternative would be to use some factory pattern to extend for more types? I can't change `AnInterface` as tgdavis suggests. I am sorry I didn't note that in the post — Jim, Oct 22 '22 at 22:11
Yep, a factory could be used as 'poor man's polymorphism' :-) — Mihe, Oct 22 '22 at 22:20
@ChengThao: this is a simplified example for clarity of the post. In reality there is a loop and adds more items within the `ifs` — Jim, Oct 23 '22 at 09:12

score 1 · Answer 1 · answered Oct 22 '22 at 22:05

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;
    }
}

Not much you can do other than `instanceof` then, but make sure that the result when a class other than Foo or Bar is passed is what you want: Throw an exception, or add an unmodified Person to the list. — tgdavies, Oct 22 '22 at 22:23

score 1 · Answer 2 · answered Nov 10 '22 at 13:34

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.

score 0 · Answer 3 · answered Oct 22 '22 at 22:18

0

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

answered Oct 22 '22 at 22:18

Federico Beach

65
4

As `Foo` is not declared abstract, it _has_ to have an implementation of `AnInterface`. – Mihe Oct 22 '22 at 22:23

Alexander Ivanchenko · Answer 4 · 2022-10-22T23:13:51.227

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);
}

Generic method using instanceof and can it be avoided?

4 Answers4

Linked