Spring MVC login controller

Question

I am making a small project in Spring MVC. My login controller looks like this:

@Controller
public class LoginController 
{ 
 @Autowired
 private Environment env;
 private static final Logger LOGGER = LoggerFactory.getLogger(LoginController.class);
 private AuthenticationService authenticationService = AuthenticationService.getAuthenticationInstance();
 @RequestMapping(value = "/login.jsp" , method = RequestMethod.GET)
 public ModelAndView userLoginPage(){
 return new ModelAndView("Login"); 
 }
 @RequestMapping(value = "/login", method = RequestMethod.POST)
 public ModelAndView userLogin(@ModelAttribute LoginModel loginModel) {
 LOGGER.info("Employee trying to login [{}] ", loginModel.toString());
 ModelAndView loginView = new ModelAndView();
 loginView.addObject("loginModel", loginModel);
 String login = authenticationService.authenticateUser(loginModel,env);
 switch (login) {
 case "Authenticated":
 loginView.setViewName("dashboard");
 break;
 case "Authenticated First time Login":
 loginView.setViewName("changepassword");
 break; 
 case "Not Authenticated":
 loginView.setViewName("Login");
 loginView.addObject("invalid", "invalid user/password");
 break;
 }
 return loginView;
 }
 @RequestMapping(value = "/changePassword" , method = RequestMethod.POST)
 public ModelAndView userPasswordChange(@ModelAttribute 
 PasswordChangeModel passwordChangeModel){
 LOGGER.info("Employee trying to change password [{}] 
 ",passwordChangeModel.getUserid());
 authenticationService.changePassword(passwordChangeModel, env);
 return new ModelAndView("passwordchangesuccess");
 }
 }

I am using switch case to delegate to different views. Is this the right approach or should I do something else to delegate response to views?

Answer 1

The switch has a bad reputation and is usually seen as a code smell.

In your case, I would first propose to replace the strings by one enum so that you have strong typing, you avoid typos errors and your Ide can warn you if you miss a case in your switch. By the way, it is a good practice to have a default case to handle and spot the unexpected cases (I usually throw a UnsupportedOperationException or IllegalArgumentException). You should of course avoid to have some MVC part into your AuthenticationService but that is a job for the adapters and his friends.

If you really want to remove this switch (I wish) then the idea is to use polymorphism. Basically, your AuthenticationService should return a subtype of a basic result class (LoginSuccess, LoginFailed) where the view name will be defined or computed.

About your controller itself, you should use @Autowired for Environment and AuthenticationService. Also note that Spring discourage the usage of autowired fields outside of tests classes.