4
\$\begingroup\$

In a project I have a few long switch statements which seem ugly to me. Please suggest how to refactor the following code.

    protected void processRequest(HttpServletRequest request,
        HttpServletResponse response) throws ServletException, IOException {
     String username = request.getParameter("username");
     String password = request.getParameter("password");

     switch(username) {
     case "client1":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app1/apk-1-index.html");
         }
         break;

     case "client2":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app2/apk-2-index.html");
         }
         break;

     case "client3":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app3/apk-3-index.html");
         }
         break;

     case "client4":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app4/apk-4-index.html");
         }
         break;
     case "client5":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app5/apk-5-index.html");
         }
         break;
     case "client6":
         if(PASSWORD.equals(password)) {
             request.getSession(true).setAttribute("username", username);
             response.sendRedirect("app5/apk-5-index.html");
         }
         break;
    default:
        response.sendRedirect("/Test2/index.jsp");
     }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Are these two switch statements related in some way? If not I would recommend posting two separate questions. I'm also wondering if you have changed the code in some way before posting it here. What is real and what did you "obfuscate"? \$\endgroup\$ – Simon Forsberg May 10 '17 at 20:28
  • \$\begingroup\$ Thanks for your answer. I will follow your suggestion and separete this post into two questions because these switch statements are unrelated. I don't understand what do you mean telling if I had changed the code - I am the owner of the code. If you are worried or puzzled about security reasons then don't be because it's not public project. \$\endgroup\$ – miki May 10 '17 at 20:38
  • \$\begingroup\$ My concern is if the code you have posted here is your real, actual code. Does the code you have actually say response.sendRedirect("app1/apk-1-index.html"); and response.sendRedirect("app2/apk-2-index.html"); and so on? And does it really say client1, client2, etc or did you examplify it? Please make it clear. \$\endgroup\$ – Simon Forsberg May 10 '17 at 20:40
  • 1
    \$\begingroup\$ Note that while some things can be examplified, if you examplify too much that might leave us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? (Note that at the moment I don't consider this off-topic for this reason) \$\endgroup\$ – Simon Forsberg May 10 '17 at 20:41
  • 1
    \$\begingroup\$ Yes it's the same. \$\endgroup\$ – miki May 10 '17 at 20:42
2
\$\begingroup\$

What is common?

Let's take a look at the cases that look similar and see what is common among them.

case <THIS PART IS DIFFERENT>:
if(PASSWORD.equals(password)) {
    request.getSession(true).setAttribute("username", username);
    response.sendRedirect(<THIS PART IS DIFFERENT>);
}

Okay, so we have essentially that one string (username) leads to another string (a redirect). One thing is pointing at another. Oh, a map/dictionary!

Map<String, String> redirects = new HashMap<>();
redirects.put("client1", "app1/apk-1-index.html");
redirects.put("client2", "app2/apk-2-index.html");
redirects.put("client3", "app3/apk-3-index.html");
....

(Yes, you could use a simple for-loop to setup this)

Then we have to determine first of all: Is our username in the map or not? If it isn't, that corresponds to our default case.

String redirect = redirects.get(username);
if (redirect == null) {
    response.sendRedirect("/Test2/index.jsp");
    return;
}

So, if it is in the map, then let's do the common things and use the redirect we picked from the map:

if (PASSWORD.equals(password)) {
    request.getSession(true).setAttribute("username", username);
    response.sendRedirect(redirect);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ So I guess for other simillar situations (with long switch statements) I could use map structure to make code look cleaner? Anyway - thanks for your help. \$\endgroup\$ – miki May 10 '17 at 20:56
  • \$\begingroup\$ @MariuszMikołajczak Very often a map does the trick, yes. \$\endgroup\$ – Simon Forsberg May 10 '17 at 21:00

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.