3

Here is my problem. I have currently a function called by a route that works properly ! However, I would like to factorize the checking parameter block.

Let me explain. When the user enters the URL to reach the function, he can put some optional parameters (6 in total). At least one of these parameters is needed to continue. My framework is configured to assign a null value to parameters that haven't been informed by user.

To check which parameters have been informed, and verify them, I have a block :

public Result edit(String param1, String param2, String param3, String param4, String param5, String param6) {

    Map<String, Object> parameters = new HashMap<>();
    if (param1 != null) {
         // Checking function depending on data type (URL, Boolean, ..), return a clean param or throw an InvalidParamException
         // Variable param depends on type returned by checkParamType1
         param = checkParamType1(param1);
         parameters.put("param1", param);
    }
    if (param2 != null) {
         param = checkParamType1(param2);
         parameters.put("param2", param);
    }
    if (param3 != null) {
         param = checkParamType2(param3);
         parameters.put("param3", param);
    }
    if (param4 != null) {
         param = checkParamType2(param4);
         parameters.put("param4", param);
    }
    if (param5 != null) {
         param = checkParamType3(param5);
         parameters.put("param5", param);
    }
    if (param6 != null) {
         param = checkParamType3(param6);
         parameters.put("param6", param);
    }
    assert(parameters.size() > 0, "At least one parameter required");

    // [TREATMENT]
}

My question is, in your opinion, is it possible to factorize this block ?

My project's running on JAVA 8.

Thank you :)

Improve this question
6
  • 2
    Can you explain what factorize means? Commented Aug 3, 2016 at 15:50
  • a good resource winterbe.com/posts/2015/03/15/avoid-null-checks-in-java Commented Aug 3, 2016 at 15:51
  • What does the methods checkParamType[123] do ? Are they in any way specific for the different arguments? Commented Aug 3, 2016 at 15:51
  • Make your method take varargs instead of 6 parameters and iterate with a loop over that. public Result edit(String... params) { for (String param : params) // and the rest of your logic Or you could even make a stream, filter it, map and collect - no if statments and no loops. Commented Aug 3, 2016 at 15:54
  • 2
    Optional in Java8 would definitely make your code look more readable... Commented Aug 3, 2016 at 15:58

5 Answers 5

Reset to default
6

The best you can do IMHO is to extract a method and use a method reference for your custom checker. Something like this:

private void checkAndAssign(Map<String, Object> map, String paramName, String paramValue, Function<String, Object> checker){
  if(paramValue!=null){
    Object param = checker.apply(paramValue);
    map.put(paramName, param);
  }
}

public Result edit(String param1, String param2, String param3, String param4, String param5, String param6) {

    Map<String, Object> parameters = new HashMap<>();
    checkAndAssign(map, "param1", param1, MyClass::checkParam1);
    checkAndAssign(map, "param2", param2, MyClass::checkParam2);
    checkAndAssign(map, "param3", param3, MyClass::checkParam3);
    checkAndAssign(map, "param4", param4, MyClass::checkParam4);
    checkAndAssign(map, "param5", param5, MyClass::checkParam5);
    checkAndAssign(map, "param6", param6, MyClass::checkParam6);
    assert(parameters.size() > 0, "At least one parameter required");

    // [TREATMENT]
}

MyClass::checkParam1 is a method reference. The compiler will allow you to substitute such a reference for a functional interface like Function.

See Method References in the Java 8 Tutorial.

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

I understand that the checkParamType method is specific for each param. So, you cannot use checkParamType1 for all parameters
Thank you @SeanPatrickFloyd I had this idea, but I did not success to implement it as I am (still) newbie. Suppose my checkParamX functions have same prototype : Object checkParamX(String paramValue, String errorMessageTemplate, Object... errorMessageArgs) My checkAndAssign method should have this prototype ? private void checkAndAssign(Map<String, Object> map, String paramName, String paramValue, Function<<String, String, List<Object>>, Object> checker) Thank you :)
@Twisky no, you should create your own functional interface which had exactly the signature you need
This design seems very wrong. It's missing a conceptual whole such as EditParams. The EditParams class should be in charge of these checks and could be designed in a DRY way. 6 individual parameters is way too much.
That doesn't matter. There are many cases where good answers surface and are highly upvoted after one has been accepted. BTW if you want to use enum, items as keys, you should be using an EnumMap, not a HashMap
|
2

In Java 8 you could replace 

if (param1 != null) {
     param = checkParamType1(param1);
     parameters.put("param1", param);
}

With something like:

Optional.ofNullable(param1)
    .ifPresent((param) -> parameters.put("param1", checkParamType1(param)));
Improve this answer

6 Comments

Absolutely valid. But do you really see a benefit here? It's just art for art's sake. The original check would be even shorter and more straightforward. if (param1 != null) parameters.put("param1", checkParamType1(param1));
@Matt It is a matter of point of view, so I let the PO decides what he prefers most.
That's a very small improvement. So small it shouldn't be considered as an answer.
@Matt your example code (if (param1 != null) parameters.put("param1", checkParamType1(param1));) probably is more clear that any answer until now. – Dherik 8 mins ago
@Twisky According to your current code your checkParamType methods seem to raise only RuntimeException so this approach is still valid. It replaces 3 lines of code with only one line of code and it gets rid of the keyword null which is probably the biggest mistake in Java programming language that has been fixed in Java 8 with the addition of Optional
|
0

At first, you could use switch case statements. But in OO languages it would be better to use a design pattern like chain of responsibility...

Improve this answer

1 Comment

Could you elaborate a bit? How would you use Chain Of Responsibility here?
0

If the number of arguments isn't limited, try using a var-args parameter and loop over the resulting array.

public Result edit(String... params) {
    Map<String, Object> parameters = new HashMap<>();
    IntStream.range(0, params.length).forEach(idx -> {
        String param = params[idx];
        if (param != null) {
            Object p = checkParamType(param);
            parameters.put("param" + idx, p);
        }
    });
}
Improve this answer

2 Comments

You didn't consider that he has multiple checkParamType() methods
It's an interesting solution, but as @Matt said, you didn't consider that I've multiple checkParamType methods :) Thank you !
-4

java 8 introduced a new feature to avoid null checks. Have a look: http://winterbe.com/posts/2015/03/15/avoid-null-checks-in-java/

Improve this answer

1 Comment

Please avoid link-only answers. They tend to get completely useless once the link breaks.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.