CODEWITHSUNDEEP
javaspringjava-8option-type
user6440081
user6440081

Reputation:

Optional parameters for Request Param is a bad practise?

I'm using Optional parameters for my Rest controller, in my case to distinguish which method to call:

@GetMapping("/cars")
@ResponseBody
public List<CarsDTO> getAllCarsByCat(@RequestParam Optional<Integer> cat1,
                                     @RequestParam Optional<Integer> cat2) {
    if (cat1.isPresent() && cat2.isPresent())
        return carsService.getAllCarsByCat1AndCat2(cat1.get(), cat2.get());
    else if (cat1.isPresent())
        return carsService.getAllCarsByCat1(cat1.get());
    else if (cat2.isPresent())
        return carsService.getAllCarsByCat2(cat2.get());
    else
        return carsService.getAllCars();

}

Why does the highest voted response of the thread below propose that "Using Optional parameters causing conditional logic inside the methods is literally contra-productive."?

Why should Java 8's Optional not be used in arguments

I do exactly that and consider it as the most readable and straight forward solution. What is bad with this approach?

Upvotes: 10

Views: 12427

Answers (1)

Ryuzaki L
Ryuzaki L

Reputation: 40068

The only concern using Optional as @RequestParam is performance and creating Optional wrapper using Optional.OfNullable and unwrapping using Optional.get() or checking using Optional.ifPresent(). As a functional perspective it always looks good using Optional but as a good programmer it is an unnecessary additional operations wrapping and unwrapping. In spring Optional is allowed in @RequestParam to declare param as optional

By default, method parameters that use this annotation are required, but you can specify that a method parameter is optional by setting the @RequestParam annotation’s required flag to false or by declaring the argument with an java.util.Optional wrapper.

So you can simply make those @RequestParam as optional using required==false and follow the same approach using if else, you can also use Objects.nonNull for more readability or you can also use defaultValue 

@GetMapping("/cars")
@ResponseBody
 public List<CarsDTO> getAllCarsByCat(@RequestParam(name="cat1", required=false) Integer cat1,
                                 @RequestParam(name="cat2", required=false) Integer cat2) {
     if (cat1!=null && cat2!=null)
         return carsService.getAllCarsByCat1AndCat2(cat1, cat2);
     else if (cat1!=null)
         return carsService.getAllCarsByCat1(cat1);
     else if (cat2!=null)
         return carsService.getAllCarsByCat2(cat2);
     else
         return carsService.getAllCars();

  }

Upvotes: 11

Related Questions