Generic method using instanceof and can it be avoided?-CodePudding

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

CodePudding user response：

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

CodePudding user response：

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

CodePudding user response：

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 separate well-readable 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);
}