1
\$\begingroup\$

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?

\$\endgroup\$
0
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.