Java semantics - Is there a way to write this better?

Question

I'm building a Spring backend. I've got a controller which gets a "search object" - an object with like 10 fields which only one of them should be filled, so the search function (which I did not write but need to make changes to and refactor) is written like this:

if( param1 != null ) user = getUserByParam1(param1);
else if ( param2 != null ) user = getUserByParam2(param2);
.
.
.
else if(lastName != null || lastName != null) user = getUserByName(firstName, lastName);
else user = getUserById(id);

if(user == null) throw costumException;
return user;

Notice the 2 special cases in the end- one of them checks the availability of 2 parameters instead of one and sends them both to same function (which can handle null in one of the fields, but not both), and the default case which assumes an ID is being passed (in case its not- it gets handled by the if (user == null) check after to throw exception).

Is there a way to refactor this piece of code to be more readable/good looking? Is there any design pattern or a known method I can use to do this? Or is it actually the best way to write this kind of functionality?

I've done a lot of thinking but couldn't find a nicer way to do so. I've had an idea to somehow send the field name that is filled and its value to another function which will "switch-case" on the field name and send the value to the appropriate function, but it doesn't really save much code (as I still need to manually iterate on all fields to find the one which is filled) and I'm not so sure it'll more readable.

I'm also quite new to Java so I do not know all the available APIs and interfaces, maybe there is something you can throw at me for help.

Answer 1

Note: This is simply a workaround for your quite monstrous method. As mentioned in the comments, your endpoint tries to do too much. It would be much better to create many different endpoints which all only need the required parameters. Makes it a lot easier to understand than just to see 10+ if-else statements

---

You could create a data-class containing all the possible parameters:

public class UserRequest {
    private String lastName;
    private String firstName;

    // more fields, constructors, getters, setters etc.
}

Then have an interface looking something like this:

public interface UserRequestResolver {
    User resolve(UserRequest request);
}

This interface can then be implemented to depend on a given param, if it exists. Return a found User or simply null.

Next step is, create a List<UserRequestResolver> resolvers, and add the different implementations, with Java8+ you can use lambdas:

resolvers.add(r -> r.getParam1() != null ? getUserByParam1(r.getParam1()) : null);
resolvers.add(r -> {
    if(r.getFirstName() == null && r.getLastName()) return null;
    return getUserByName(r.getFirstName(), r.getLastName());
});
// etc.

Then when receiving a UserRequest you can simply iterate over resolvers and take the first return value which is not null:

for(UserRequestResolver resolver : resolvers) {
    User user = resolver.resolve(request);
    if(user != null) return user;
}
throw costumException;

If you're a fan of Streams, then you can replace the for-loop above with this:

return resolvers.stream()
    .map(resolver -> resolver.resolve(request))
    .filter(Objects::nonNull)
    .findFirst()
    .orElseThrow(() -> costumException);

Answer 2

Depending on what you use to access the data, you can use getByExample query. You have an example of using it here with Spring data. With this you only have to get the user class from the api (from you api you must receive and object for the search query, this object must be somewhat like the user object as the parameters looks alike).

And then where you have your big if/else you pass the user search object from the api and in the repository do a getByExample.

user = getByExample(userSearchObject)

But this is all assuming you receive in your api only one field filled in your object. This would be the most concise way to do it.

Otherwise I would suggest wrapping all your parameter into a class in the api like this:

public class UserSearchQuery() {
    private String param1;
    private String param2;
    ...
    
    // getters + setters
}

and then use criteria query and criteria query builders (An article about it here).

In your service you would only have to do this:

public class UserService() {

private UserRepository repository;
   public User search(UserSearchQuery query) {
      return repository.search(query);
   }
}

and in the repo :

public class UserRepository() {
   private EntityManager em;
   
   public User search(UserSearchQuery query) {
      CriteriaBuilder builder = em.getCriteriaBuilder();
      CriteriaQuery<User> query = builder.createQuery(User.class);

      Root<User> user = query.from(User.class);
      List<Predicate> predicates = new ArrayList<>();
     
      if (query.getParam1() != null) {
          predicates.add(builder.equal(user.get("param1"), query.getParam1()));
      }
      ...
      // and the other ones goes here
   }
}

Doing so you would only have 1 search method for your User and no longer 10 methods, one for each parameters, and to add a search criteria you would only have to add a new parameter to the UserSearchQuery class and add a new predicate. Also if someday you would like to be able to use multiple parameters as search criteria, it would already be done.

Answer 3

You can send all the fields in a request parameter and then use command design pattern or strategy design pattern to implement different implementation for different case.

Example for command pattern is as below:

https://sourcemaking.com/design_patterns/command

Strategy design pattern: https://sourcemaking.com/design_patterns/strategy