Is using if/else if/else this way a good practice?

Question

I am working on an app we inherited from a previous vendor and many places in the client side javascript I see something like this:

if (self.DESIGNEE_TYPE_SCREEN_ROUTE_REGEX.test(url)) {
    goToSelectDesigneeType.call(self);
} else if (self.FUNCTION_CODES_SCREEN_ROUTE_REGEX.test(url)) {
    goToSelectFunctionCodes.call(self);
} else if (self.DISCIPLINE_SCREEN_ROUTE_REGEX.test(url)) {
    goToSelectDiscipline.call(self);
} else {
    goToSelectDesigneeType.call(self);
} else ...

This is used to to go to the next step in a workflow, and it works fine but something about it just does not sit right with me. Is this pretty common, or is there a better way to do the same thing?

If this is working code, you might be better off in http://codereview.stackexchange.com — jfriend00, Mar 05 '15 at 04:50
Switch statement is much better. Also you can only have one else in a if / if else / else block. — pmac89, Mar 05 '15 at 04:50
I think this will help you. http://stackoverflow.com/questions/2922948/javascript-switch-vs-if-else-if-else — Saurin Dashadia, Mar 05 '15 at 04:54
You could create an array with each element comprising a regular expression and the function to call and then iterate over that until you find a match; but I doubt that will be worth the effort. — Ja͢ck, Mar 05 '15 at 05:14
How do your regexes look like? You [might be able to optimise](http://stackoverflow.com/a/10029116/1048572) if they follow a trivial pattern. Otherwise follow the suggestion of @Ja͢ck. — Bergi, Mar 05 '15 at 05:18

score 1 · Accepted Answer · answered Mar 05 '15 at 05:21

You can use switch(true) idiom here.

switch(true) {
  case self.DESIGNEE_TYPE_SCREEN_ROUTE_REGEX.test(url) :
    goToSelectDesigneeType.call(self);
    break;
  case self.FUNCTION_CODES_SCREEN_ROUTE_REGEX.test(url) :
    goToSelectFunctionCodes.call(self);
    break;
  case self.DISCIPLINE_SCREEN_ROUTE_REGEX.test(url) :
    goToSelectDiscipline.call(self);
    break;
  default :
    goToSelectDesigneeType.call(self);
}

just my opinion though, this is much easier to read.

Is using if/else if/else this way a good practice?

1 Answers1