Yuriy
Yuriy

Reputation: 1442

How to implemenet correct interaction between Controller and Service layer in Spring MVC

I have designed web app with Spring MVC, Spring Security and Hibernate. I have controller which interacts with Service layer:

@Controller
@RequestMapping(value="/")
public class InitController {

    @Autowired
    private UserService userService;


    @Autowired
    private StudentService studentService;

    @InitBinder
    public void initBinder(WebDataBinder dataBinder){
        dataBinder.registerCustomEditor(String.class, "studentGroup", new StudentNameEditor());
    }

    @RequestMapping(value = "/login", method = RequestMethod.GET)
    public ModelAndView getLoginForm(){
        return new ModelAndView("login");
    }

    @RequestMapping(value = "/registration.html",method = RequestMethod.GET)
    public ModelAndView getRegistrationForm(){
        return new ModelAndView("registration");
    }

    @RequestMapping(value = "/students.html",method = RequestMethod.GET)
    public ModelAndView getGroupForm(){
        return new ModelAndView("searchStudents");
    }


    @RequestMapping(value = "/getStudents.html",method = RequestMethod.POST)
    public ModelAndView getStudents(@Valid @ModelAttribute("student") Student student,
                                BindingResult result){
        if(result.hasErrors()){
            return new ModelAndView("searchStudents");
        }else{
            return studentService.getStudentOfGroup(student);
        }

    }

    @RequestMapping(value = "/registrationConfirm.html",method = RequestMethod.POST)
    public ModelAndView registration(@Valid @ModelAttribute("user") User user,
                                BindingResult result){
        if(result.hasErrors()){
            return new ModelAndView("registration");
        }else{
            User savedUser = userService.registerUser(user);
            if(Objects.isNull(savedUser)){
                ModelAndView modelAndView = new ModelAndView("login");
                modelAndView.addObject("resultRegistration", 
                                        "Success registration!");
                return modelAndView;
            }else{
                ModelAndView modelAndView = new ModelAndView("registration");
                modelAndView.addObject("resultRegistration", 
                                        "User with the same login or password is registered in system already");
                return modelAndView;
            }
        }
    }
}

Service layer interacts with DAO layer:

@Transactional(propagation = Propagation.REQUIRES_NEW )
@Service("userService")
public class UserServiceImpl implements UserService {

    @Autowired
    private UserDAO userDao;

    @Transactional(propagation = Propagation.REQUIRES_NEW )
    @Override
    public User registerUser(User user) {
        User userWithTheSameLogin = userDao.getUserByLogin(user.getUserLogin());
        if(!Objects.isNull(userWithTheSameLogin)){
            //if user with the same login registered already
            return userWithTheSameLogin;
        }else{
            User userWithTheSamePassword = userDao.getUserByEmail(user.getUserEmail());
            if(!Objects.isNull(userWithTheSamePassword)){
                //if user with the same email registered already
                return userWithTheSamePassword;
            }
            else{
                //if user's credentials are unique
                userDao.saveUser(user);
                return null;
            }
        }

    }
}

I return user from Service layer, when email or login existed, and null when saving was successfull.

I think that simple returning value - it is bad, It is not logic and not distinctly. Advice me please the best way for notifying Controller about result of saving user. It is throwing exception and catching them in controller or returning Status code from Service layer? And show example please. I will be very grateful

Upvotes: 0

Views: 786

Answers (1)

Roel Strolenberg
Roel Strolenberg

Reputation: 2950

My suggestion is returning a boolean where registerUser is true if the user can be registered and false otherwise:

@Transactional(propagation = Propagation.REQUIRES_NEW )
@Override
public boolean registerUser(User user) {
    User userWithTheSameLogin = userDao.getUserByLogin(user.getUserLogin());
    if(!Objects.isNull(userWithTheSameLogin)){
        //if user with the same login registered already
        return false;
    }
    User userWithTheSamePassword = userDao.getUserByEmail(user.getUserEmail());
    if(!Objects.isNull(userWithTheSamePassword)){
        //if user with the same email registered already
        return false;
    }
    //if user's credentials are unique
    userDao.saveUser(user);
    return true;
}

In your controller you would then have:

@RequestMapping(value = "/registrationConfirm.html",method = RequestMethod.POST)
public ModelAndView registration(@Valid @ModelAttribute("user") User user,
                            BindingResult result){
    if(result.hasErrors()){
        return new ModelAndView("registration");
    }
    if(userService.registerUser(user)){
        ModelAndView modelAndView = new ModelAndView("login");
        modelAndView.addObject("resultRegistration", "Success registration!");
        return modelAndView;
    }
    //If the other cases didn't hold true, it means the registration failed
    ModelAndView modelAndView = new ModelAndView("registration");
    modelAndView.addObject("resultRegistration", 
        "User with the same login or password is registered in system already");
    return modelAndView;
}

This feels more readable and gets rid of null checks. Hope this feels "better" to you as well.

Upvotes: 1

Related Questions