1

I have defined a REST GET API which will take all the Request Parameters and returns the result based on the values of request parameters. Something like below:

@GetMapping(produces = MediaType.APPLICATION_JSON_VALUE)
public List<Student> findAll(@RequestParam Map<String, String> allRequestParams) {
    if(allRequestParams.isEmpty()) {
        return studentRepository.findAll();
    } else {
       return studentService.findWithFilter(allRequestParams); // This method will take care of using appropriate findBy method on the repository
    }
}

The above code is using Spring way of retrieving all request parameters. I could instead inject HttpServletRequest and use getParametersMap() method to get all the request parameters. I have few questions:

  1. Is it better to use Spring way or use getParameterMap() from HttpServletRequest?
  2. I was told in one of the code review that this kind of programming is anti-pattern? Can somebody shed some light on why it is considered as anti-pattern? And if it is an anti-pattern, then what is the best way to handle this kind of processing?
5
  • The method name is a lie. It doesn't find all if you provide a filter. A findAll shouldn't accept any filter. If you want such filterable find, then add something like findById or findByName. Commented Jan 28, 2017 at 12:56
  • Thanks @Tom. Agreed, but this is not a production code, and please ignore the naming conventions used here. Commented Jan 28, 2017 at 12:58
  • which kind of programming...injection httpservletrequest ?? Commented Jan 28, 2017 at 13:27
  • Spring can automatically inject a number of types into controller methods, like Model, Authentication, Session, HttpServletRequest and others - and you can build your own providers if you like. Commented Jan 28, 2017 at 16:23
  • @KlausGroenbaek - I don't understand how is your comment related to my question. Can you please explain? Commented Jan 28, 2017 at 19:42

2 Answers 2

1
  1. The Spring way is better since it's more convenient. The only reason you would want to use HttpServletRequest instead would be if you wanted to only tie your code to non-vendor APIs. Since you're already using Spring, you might as well use it for all its worth.

    You might want to consider using your own object to hold the request parameters. That makes it easier for other developers to see what query parameters that's supported by your API. I.e.

    @GetMapping(produces = MediaType.APPLICATION_JSON_VALUE)
    public List<Student> findAll(@RequestParam MyQueryObject allRequestParams) {
        ...
    }
    

    Spring will map query parameters onto MyQueryObject's fields, and will return sensible error messages to the client if that's not possible. Spring will also base the format of the error message based on the Accept-header so that clients that expects application/json will get that, and clients expecting text/html gets that.

  2. The only reason it would be an anti-pattern is that it is weakly typed. You might say that is an anti-pattern in Java as Java favours static types.

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

Comments

0

Better not to inject HttpServletRequest in your api service since HttpServletRequest should ideally be only used in the web layer. Letting spring give you the data you need to perform your functionality is the best way since you are then not dependent for HttpServletRequest to be set to create an object of your service and thus if you want to test your service you can pass a map of strings while testing instead of having a dependency on the HttpServletRequest object for your unit testing

2 Comments

He is basically saying that the solution you already have is better than injecting the HttpServletRequest and calling getParametersMap(). Mainly because you should stop using Web specific stuff after the HttpMessageConverter layer. Using @RequestParam means that the Controller method can be tested without having to Mock a HttpServletRequest``.
I was answering your first question.

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.