Difference between service layer and controller in practice

Question

I have read many theories about differences between service layer and controller, and I have some questions about how to realize this in practice. One answer to Service layer and controller: who takes care of what? says:

I try to restrict controllers to doing work related to validating http parameters, deciding what service method to call with what parameters, what to put in the httpsession or request, what view to redirect or forward to, or similar web-related stuff.

and from http://www.bennadel.com/blog/2379-a-better-understanding-of-mvc-model-view-controller-thanks-to-steven-neiland.htm :

Red Flags: My Controller architecture might be going bad if:

The Controller makes too many requests to the Service layer. The Controller makes a number of requests to the Service layer that don't return data. The Controller makes requests to the Service layer without passing in arguments.

At the moment I am developing a web app with Spring MVC, and I have such method for saving changed user's email:

/**
     * <p>If no errors exist, current password is right and new email is unique,
     * updates user's email and redirects to {@link #profile(Principal)}
     */
    @RequestMapping(value = "/saveEmail",method = RequestMethod.POST)
    public ModelAndView saveEmail(
            @Valid @ModelAttribute("changeEmailBean") ChangeEmailBean changeEmailBean,
            BindingResult changeEmailResult,
            Principal user,
            HttpServletRequest request){

        if(changeEmailResult.hasErrors()){
            ModelAndView model = new ModelAndView("/client/editEmail");
            return model;
        }
        final String oldEmail = user.getName();
        Client client = (Client) clientService.getUserByEmail(oldEmail);
        if(!clientService.isPasswordRight(changeEmailBean.getCurrentPassword(), 
                                          client.getPassword())){
            ModelAndView model = new ModelAndView("/client/editEmail");
            model.addObject("wrongPassword","Password doesn't match to real");
            return model;
        }
        final String newEmail = changeEmailBean.getNewEmail();
        if(clientService.isEmailChanged(oldEmail, newEmail)){
            if(clientService.isEmailUnique(newEmail)){
                clientService.editUserEmail(oldEmail, newEmail);
                refreshUsername(newEmail);
                ModelAndView profile = new ModelAndView("redirect:/client/profile");
                return profile;
            }else{
                ModelAndView model = new ModelAndView("/client/editEmail");
                model.addObject("email", oldEmail);
                model.addObject("emailExists","Such email is registered in system already");
                return model;
            }
        }
        ModelAndView profile = new ModelAndView("redirect:/client/profile");
        return profile;
    }

You can see that I have a lot of requests to the service layer, and I do redirecting from controller - that is business logic. Please show better version of this method.

And another example. I have this method, which returns user's profile:

/**
     * Returns {@link ModelAndView} client's profile
     * @param user - principal, from whom we get {@code Client}
     * @throws UnsupportedEncodingException
     */
    @RequestMapping(value = "/profile", method = RequestMethod.GET)
    public ModelAndView profile(Principal user) throws UnsupportedEncodingException{
        Client clientFromDB = (Client)clientService.getUserByEmail(user.getName());
        ModelAndView model = new ModelAndView("/client/profile");
        model.addObject("client", clientFromDB);
        if(clientFromDB.getAvatar() != null){
            model.addObject("image", convertAvaForRendering(clientFromDB.getAvatar()));
        }
        return model;
    }

method convertAvaForRendering(clientFromDB.getAvatar()) is placed in super class of this controller, it is right placing of this method, or he must be placed in service layer??

Help please, it is really important for me.

Answer 1

A Spring Controller is generally tied to the Spring API (with classes like Model, ModelAndView...) or the Servlet API (HttpServletRequest, HttpServletResponse...). Methods can return String outcomes that are resolved to the name of a template (JSP...). Controller are most certainly biased toward Web GUIs, with a strong dependence upon Web technology.

Services on the other hand should be designed with business logic in mind and no assumptions about the client. We could remote the service, expose it as Web Service, implement a Web front end, or a Swing client. A Service should not depend upon Spring MVC, Servlet API, and the like. That way, if you need to retarget your application, you could reuse most of the business logic.

As for the note concerning too many calls to the service layer from the controller layer, it mostly is a matter of performance, which IMHO is something different. If each call to the service layer queries a database, you might experience performance issues. It the service layer and the controller layer are not running in the same JVM, you could experience performance issues either. This is another very important aspect of designing your application, but it would indicate that you should façade your services calls to provide coarser-grained operations to the controller layer.

Answer 2

In both examples, why do you need to cast Client? That's a code smell.

Since the call to the service tier is also the call that establishes the database transaction boundary, making multiple calls means that they are executed in different transactions, and are therefore not necessarily consistent with each other.

That is one of the reasons why multiple calls are discouraged. @ArthurNoseda mentions other good reason in his answer.

In your first case, there should be a single call to the service tier, e.g. something like this:

if (changeEmailResult.hasErrors()) {
    return new ModelAndView("/client/editEmail");
}
try {
    clientService.updateUserEmail(user.getName(),
                                  changeEmailBean.getCurrentPassword(),
                                  changeEmailBean.getNewEmail());
} catch (InvalidPasswordException unused) {
    ModelAndView model = new ModelAndView("/client/editEmail");
    model.addObject("wrongPassword", "Password doesn't match to real");
    return model;
} catch (DuplicateEmailException unused) {
    ModelAndView model = new ModelAndView("/client/editEmail");
    model.addObject("email", oldEmail);
    model.addObject("emailExists", "Such email is registered in system already");
    return model;
}
refreshUsername(newEmail);
return new ModelAndView("redirect:/client/profile");

You could also use return value instead of exceptions.

As you can see, this will delegate the business logic of changing the email to the service tier, while keeping all UI related actions in the controller where they belong.