3

I had a service that I was doing a lot of if/else statements so I found that using Strategy Pattern could help me a lot.

I am working with Mongo and I need to store two different models in the database in different collections. These models extends a common that have some similar information. I started deciding in runtime what class it would be used, but right now my code is showing a warning: unchecked call to 'save(T)' as a member of type 'Strategy'.

Below is the code I'm trying to do. For now, I just added a @SuppressWarnings("unchecked") and it's working so far, but I'd like to confirm if this might cause problems and what would be a better approach.

Question: How could I fix this?

Code as of now:

interface Strategy<T extends Person> {
    T save(T model);
    //other methods
}

class StudentStrategy implements Strategy<Student> {
    private StudentRepository studentRepository;

    public Student save(Student student) {
        return studentRepository.save(student);
    }
}
class TeacherStrategy implements Strategy<Teacher> {
    private TeacherRepository teacherRepository;

    public Teacher save(Teacher teacher) {
        return teacherRepository.save(teacher);
    }
}

class PersonService {
    void doSomething(Person person) {
        Strategy strategy = StrategyFactory.getStrategy(person.getType());
        strategy.save(person); //THIS LINE SAYS A unchecked call to 'save(T)' as a member of type 'Strategy'
    }
}

class StrategyFactory {
    public static Strategy getStrategy(PersonType personType) {
        if(personType == STUDENT) return new StudentStrategy();
        if(personType == TEACHER) return new TeacherStrategy();
        return null;
    }
}

Old Code:

This is the code I was doing previously. As you can see, there are a lot of if/else. Changing the repository to store Person is not an options as I need the information of each sub class..

class OldPersonService {
    void doSomething(Person person) {
        if(person.getType == STUDENT) {
            studentRepository.save(person);
        } else {
            teacherRepository.save(person);
        }
        person.setBirthday(new Date());
        person.setName("john");
        if(person.getType == STUDENT) {
            studentRepository.findReferals(person);
        } else {
            teacherRepository.findReferals(person);
        }
    }
}
Felipe S.
  • 1,633
  • 2
  • 16
  • 23
  • 1
    See e.g. https://stackoverflow.com/q/2770321/2891664 and https://docs.oracle.com/javase/tutorial/java/generics/rawTypes.html. It's a bit difficult to suggest a solution, though, without knowing more about what you're doing. Actually, the more conventional solution here is not use generics or strategy at all, but to write an abstract method on the `Person` class which `Student` and `Teacher` override. Another possibility could be like what's pictured in my answer here: https://stackoverflow.com/a/44422954/2891664. – Radiodef Jul 07 '17 at 18:10
  • hi @Radiodef ! I tried using abstract, the problem is that if I do so, my repository wouldn't be able to return each sub class and I wouldn't be able to access each individual attribute of it. But thanks for the links, it helped to understand more about generics! :) – Felipe S. Jul 07 '17 at 18:32
  • Possible duplicate of [What is a raw type and why shouldn't we use it?](https://stackoverflow.com/questions/2770321/what-is-a-raw-type-and-why-shouldnt-we-use-it) – Tom Jul 07 '17 at 18:44

1 Answers1

4

Your factory returns a raw Strategy :

class StrategyFactory {
    public static Strategy getStrategy(PersonType personType) {
        if(personType == STUDENT) return new StudentStrategy();
        if(personType == TEACHER) return new TeacherStrategy();
        return null;
    }
}

So for the client class that uses this factory :

Strategy strategy = StrategyFactory.getStrategy(person.getType());
strategy.save(person); 

the compiler emits a warning as save() is a method designed to work with a generic type :

T save(T model);

In your case T should be Student or Teacher but you don't specify it in the factory method.

To ensure type safety for the strategy factory client class, getStrategy() should return a strategy conforms to what the client expect to have.

You could for example define a scoped method type : <T extends Person> that you use to cast the strategy :

 public static <T extends Person>  Strategy<T> getStrategy(PersonType personType) {
        if(personType == STUDENT) return (Strategy<T>) new StudentStrategy();
        if(personType == TEACHER) return (Strategy<T>) new TeacherStrategy();
        return null;
    }
 }

You can notice the downcasts in the getStrategy() method.
These are unavoidable if you want to to define a unique method that returns a Strategy instance parameterized with a generic type that changes according to the client request.
Now, it is always better to perform cast in the provider classes than in the client classes where this should be performed multiple times and could be error prone.

In this way, the client could write :

Strategy<Person> strategy = StrategyFactory.getStrategy(person.getType());
strategy.save(person); 
davidxxx
  • 125,838
  • 23
  • 214
  • 215
  • 1
    Thanks for this! I liked how you make the cast, and although it still shows a unchecked cast warning, I can put it to ignore and as you said, it's better to perform cast in the provider. Thank you very much for your help!! – Felipe S. Jul 07 '17 at 18:33
  • @GhostCat :) Oh, it is good : I can at last call you M. 50K. Sincere congratulations dear colleague :) – davidxxx Jul 07 '17 at 19:48
  • Thank you sir! Talk to you next week I guess ;-) – GhostCat Jul 07 '17 at 19:49