CODEWITHSUNDEEP
javarefactoring
Anthony
Anthony

Reputation: 35928

how to refactor this method which has multiple if/else statements

I have a feeling that this if/else should be refactored out but I'm unsure of what I can do, or whether I should just let it be as it is...

private String someReportUrl(HttpServletRequest request, HttpServletResponse response) {
    String url;
    if (isBackToReportsSummary(request)) {
        url = SUMMARY_PAGE;
        getReportsSummary(request, response);
    } else if (isComingFromPageA(request)) {
        url = getTabUrl(request, REPORT_URL_FOR_PAGE_A);
    }
    else {
        url = "/standAlone/reportUrl.jsp";
    }
    return url;
}

Basically I have a reports summary page which lists three to four reports. First if condition is when the user wants to go back to that page, second condition is for when user has selected this particular report, and the third condition is for when the user selects this report as a stand alone report (not from summary page).

Upvotes: 5

Views: 4159

Answers (4)

afrin216
afrin216

Reputation: 2335

You code is fine just the way it is. But you can also look into using ?: operator, if you want to achieve the same in a single line.

An example would be:

class round{
    public static void main(String args[]){

    int sampleInt=3;
    if(sampleInt==1){
        sampleInt = 5;
        System.out.println("One");
    }
    else if(sampleInt==2){
    sampleInt = 3;
        System.out.println("Two");
    }
    else{
        sampleInt = 4;
        System.out.println("Else");
    }

    sampleInt = sampleInt==1?5:(sampleInt==2?3:4);
    System.out.println("sampleInt "+sampleInt);
}
}

At the end your code would look something like this:

   url = isBackToReportsSummary(request)==true?SUMMARY_PAGE:(isComingFromPageA(request)==true?getTabUrl(request, REPORT_URL_FOR_PAGE_A):"/standAlone/reportUrl.jsp");

Upvotes: 0

Håvard Geithus
Håvard Geithus

Reputation: 5614

How about this "guard based" style? It often makes the method easier to read from top to bottom.

private String someReportUrl(HttpServletRequest request, HttpServletResponse response) {
    if (isBackToReportsSummary(request)) {
        getReportsSummary(request, response);
        return SUMMARY_PAGE;
    } 
    if (isComingFromPageA(request)) {
        return getTabUrl(request, REPORT_URL_FOR_PAGE_A);
    }
    return "/standAlone/reportUrl.jsp";
}

Upvotes: 5

waldyr.ar
waldyr.ar

Reputation: 15184

First take a look at the Design Pattern Command. It should refactor the if/else's responsability, making it more organized and much more maintainable. And then you code should look like this:

Example 

class ExampleServlet  {

  private HashMap commandMap = new HashMap();

  public ExampleServlet() {
    commandMap.put("create", new ActionTypeCreate());
    commandMap.put("replace", new ActionTypeReplace());
    commandMap.put("update", new ActionTypeUpdate());
    commandMap.put("delete", new ActionTypeDelete());
  } //endconstructor
} //endclass: ExampleServlet

private void performTask(String action) {
    ActionType cmd = (ActionType)commandMap.get(action);
    cmd.execute();
} //endmethod: performTask

HERE You can gather more knowledge in command pattern

Upvotes: 7

Daniel Fischer
Daniel Fischer

Reputation: 183883

If you absolutely want to change it, you could initialise url to the default return and only change it if one of the two conditions is met:

private String someReportUrl(HttpServletRequest request, HttpServletResponse response) {
    String url = "/standAlone/reportUrl.jsp";
    if (isBackToReportsSummary(request)) {
        url = SUMMARY_PAGE;
        getReportsSummary(request, response);
    } else if (isComingFromPageA(request)) {
        url = getTabUrl(request, REPORT_URL_FOR_PAGE_A);
    }
    return url;
}

But really, it's fine as is.

Upvotes: 5

Related Questions