1
\$\begingroup\$

I have an endpoint in my RestAPI which can receive 3 optional parameters and then find information in the database with these parameters, also it is possible to combine the parameters. So, in my controller I take a Map<String, Object> which contains these parameters and then pass it to the service level, where I check which parameters were passed and then I call a specific DAO method, depends on which combination of parameters do I have. Params A and B are of type String, and C is a Map. So in the service level it looks like this:

public Set<ResultEntry> getByParameters(Map<String, Object> query) {
    Set<ResultEntry> result = new HashSet<>();
    if (query.containsKey("a") && query.containsKey("b") && query.containsKey("c")) {
        result.addAll(myDao.getByABC(
                (String) query.get("a"), (String) query.get("b"),
                (Map<String, String>) query.get("c")
        ));
    } else if (query.containsKey("a") && query.containsKey("b") && !query.containsKey("c")) {
        result.addAll(myDao.getByAB(
                (String) query.get("a"), (String) query.get("b")
        ));
    } else if (query.containsKey("a") && !query.containsKey("b") && query.containsKey("c")) {
        result.addAll(myDao.getByAC(
                (String) query.get("a"), (Map<String, String>) query.get("c")
        ));
    } else if (!query.containsKey("a") && query.containsKey("b") && query.containsKey("c")) {
        result.addAll(myDao.getByBC(
                (String) query.get("b"), (Map<String, String>) query.get("c")
        ));
    } else if (query.size() == 1 && query.containsKey("a")) {
        result.addAll(myDao.getByA((String) query.get("a")));
    } else if (query.size() == 1 && query.containsKey("b")) {
        result.addAll(myDao.getByB((String) query.get("b")));
    } else if (query.size() == 1 && query.containsKey("c")) {
        result.addAll(myDao.getByC((Map<String, String>) query.get("c")));
    }
    return result;
}

Everything works fine, but I feel like this is not the best way to deal with such situations, using if-statements for each possible combination, so how can I improve it?

\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

Although it works, the major issue with this approach is its extreme complexity and concreteness. If a new parameter d enters the game, it will became hellish to maintain!

What can be done to improve the current implementation, without changing the approach?

  • there are many repetitions of .containsKey(arg) calls. They can be extracted to respective booleans like hasA, hasB -> the code will be more readable.

  • there are many redundant casts. (String) query.get("a") and the others can be extracted into respective local var a = (String) query.get("a") -> it will also become shorter and more readable.

  • I think that the result variable might be avoided, if myDao.get* methods already return Sets.

How the design can be improved?

For the solution below, I suppose that the interface of the myDao object cannot be changed. If it could, there might be ways to play with method overloads, attempting to apply a pattern.

The current API of myDao looks very concrete and clear. The names of the methods mention each expected parameter and the order and the number of the arguments is predictable for each case. So the solution that I suggest below is based on the calculation of the method to invoke by reflection.

The name of the method to invoke on myDao can be calculated as follows:

private String calculateDaoMethodToInvoke(boolean hasA, boolean hasB, boolean hasC) {
  var builder = new StringBuilder("getBy");
  if (hasA) {
    builder.append("A");
  }
  if (hasB) {
    builder.append("B");
  }
  if (hasC) {
    builder.append("C");
  }
  return builder.toString();
}

We can also calculate the arguments to invoke this method with:

private Object[] calculateDaoMethodArgs(String a, String b, Map<String, String> c) {
  return Stream.of(a, b, c)
               .filter(Objects::nonNull)
               .toArray();
}

Now, since we know the name of the target method and the parameters to use, the Java reflection features can be used to produce the results.

We can define a wrapper around the Class.getMethod call, in order to retrieve the reference to the method of myDao to be invoked:

private Method extractDaoMethod(String daoMethodName, Object[] params) {
  try {
    var paramTypes = Arrays.stream(params)
                           .map(Object::getClass)
                           .toArray(Class[]::new);
    return Dao.class.getMethod(daoMethodName, paramTypes);
  } catch (NoSuchMethodException ex) {
    throw new IllegalStateException(ex);
  }
}

And its invocation can also be wrapped:

private Collection<ResultEntry> getByParams(String daoMethodName, Object[] params) {
  var method = extractDaoMethod(daoMethodName, params);
  try {
    return (Collection<ResultEntry>) method.invoke(myDao, params);
  } catch (IllegalAccessException | InvocationTargetException ex) {
    throw new IllegalStateException(ex);
  }
}

Finally, the getByParameters method is reduced to the following:

public Set<ResultEntry> getByParameters(Map<String, Object> query) {
  var a = (String) query.get("a");
  var b = (String) query.get("b");
  var c = (Map<String, String>) query.get("c");

  var daoMethodName = calculateDaoMethodToInvoke(a != null, b != null, c != null);
  var daoArgs = calculateDaoMethodArgs(a, b, c);
  return new HashSet<>(getByParams(daoMethodName, daoArgs));
}

Please note that there are no more rigid if - else if chaining, the algo became flat.

This approach allows to easily introduce new parameters, provided that the myDao API continues to respect the same convention for its method names.

There can be further improvements. For example, the signatures of calculateDaoMethodToInvoke and calculateDaoMethodArgs can be changed in order to accept varargs (and corresponding changes in the implementation). It will make the introduction of new parameters even easier.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.