HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Refactoring multiple `else if` code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
refactoringcodeelsemultiple

Problem

I think I can refactor this:

if (clicked_id == "contain-word") {
  $("#specie_filter li.checked").removeClass("checked");
  $("#specie_filter li:first").addClass("checked")
  $("#specie_filter ul").scrollTop(0)               

  $("#cell_filter li.checked").removeClass("checked")
  $("#cell_filter li:first").addClass("checked")
  $("#cell_filter ul").scrollTop(0)

  $("#factor_filter li.checked").removeClass("checked")
  $("#factor_filter li:first").addClass("checked")
  $("#factor_filter ul").scrollTop(0)                    
} else if (clicked_id == "specie_filter") {
  $("#cell_filter li.checked").removeClass("checked")
  $("#cell_filter li:first").addClass("checked")
  $("#cell_filter ul").scrollTop(0)

  $("#factor_filter li.checked").removeClass("checked")
  $("#factor_filter li:first").addClass("checked")
  $("#factor_filter ul").scrollTop(0)

} else if (clicked_id == 'cell_filter') {
  $("#factor_filter li.checked").removeClass("checked")
  $("#factor_filter li:first").addClass("checked")
  $("#factor_filter ul").scrollTop(0)
}


it into something like

if (clicked_id == "contain-word" | clicked_id == "specie_filter"`)


but that will make the code more complex and the duplicity still exists in the conditional statement. Does anyone have ideas about refactoring this?

Solution

I know I'm late to the game, but each filter function contains essentially the same lines of code. Only the container element differs:

function processFilter($container) {
    $container.find("li.checked").removeClass("checked");
    $container.find("li:first").addClass("checked");
    $container.find("ul").scrollTop(0);
}

switch (clicked_id) {
    case "contain-word":
        processFilter($("#specie_filter"))
        // fall-through...
    case "specie_filter":
        processFilter($("#cell_filter"));
        // fall-through...
    case "cell_filter":
        processFilter($("#factor_filter"));
        // fall-through...
}


EDIT: @toto2, thanks for the reminder. Yeah, comments are useful in this case since omitting the breaks in a switch statement is not normally encountered.

Also, if you want to get a little crazy with the jQuery API, this could also work:

function processFilter(selector) {
    $(selector)
        .find("ul").scrollTop(0)
        .find("li.checked").removeClass("checked")
        .parent()
        .find("li:first").addClass("checked");
}

switch (clicked_id) {
    case "contain-word":
        processFilter("#specie_filter");
        // fall-through...
    case "specie_filter":
        processFilter("#cell_filter");
        // fall-through...
    case "cell_filter":
        processFilter("#factor_filter");
        // fall-through...
}

Code Snippets

function processFilter($container) {
    $container.find("li.checked").removeClass("checked");
    $container.find("li:first").addClass("checked");
    $container.find("ul").scrollTop(0);
}

switch (clicked_id) {
    case "contain-word":
        processFilter($("#specie_filter"))
        // fall-through...
    case "specie_filter":
        processFilter($("#cell_filter"));
        // fall-through...
    case "cell_filter":
        processFilter($("#factor_filter"));
        // fall-through...
}
function processFilter(selector) {
    $(selector)
        .find("ul").scrollTop(0)
        .find("li.checked").removeClass("checked")
        .parent()
        .find("li:first").addClass("checked");
}

switch (clicked_id) {
    case "contain-word":
        processFilter("#specie_filter");
        // fall-through...
    case "specie_filter":
        processFilter("#cell_filter");
        // fall-through...
    case "cell_filter":
        processFilter("#factor_filter");
        // fall-through...
}

Context

StackExchange Code Review Q#56741, answer score: 14

Revisions (0)

No revisions yet.