jquery / contribute.jquery.org

Developer documentation common to jQuery projects
https://contribute.jquery.org
Other
25 stars 76 forks source link

JS style guide: When to use switch statements, how to format them #52

Closed jzaefferer closed 11 years ago

jzaefferer commented 11 years ago

Afaik they are only acceptable when a fall-through is needed. Otherwise use if/else.

Reference for formatting:

switch ( event.keyCode ) {
case $.ui.keyCode.ENTER:
case $.ui.keyCode.SPACE:
    z();
    break;
case $.ui.keyCode.ESCAPE:
    y();
    break;
default:
    x();
}
scottgonzalez commented 11 years ago

Is this a rule we want to keep? We have code that violates this today, even the sample code above doesn't follow the rules.

dmethvin commented 11 years ago

Core doesn't use switch anywhere at the moment, but in general I agree they're most useful when you need the fallthrough. I seem to recall there being some perf issues on older browsers, dunno if that's true today.

scottgonzalez commented 11 years ago

The question really boils down to do we want to disallow code like the above and require repeated if statements for things like keypress handlers?

dmethvin commented 11 years ago

There should be some guidelines on when switch is the right solution, and the example above would not fit the guidelines IMO because an if could do the job. :smile_cat: I could see the argument if there were bigger sets of checks though.

scottgonzalez commented 11 years ago

Ok, well I don't see any uses of switch in UI or Mobile that actually have bodies in the fall throughs, and I'm having a hell of a time finding any actual examples of fall throughs searching on Google. Perhaps we should just ban switch from jQuery projects? I personally like them, but it sounds like all current uses will need to go away anyway.

dmethvin commented 11 years ago

I can see the use of switch for large cases like this one in datepicker, we just need to come up with some guidelines for when to use them. Again, the reference sample seems too trivial to justify one.

jzaefferer commented 11 years ago

@tjvantoll you've been working the datepicker rewrite. Any opinions on the switch statement? Do we still need it there?

@rxaviers you've been working on the Globalize CLDR rewrite, with lots of parsing code. Do those justify or even require using switch statements?

rxaviers commented 11 years ago

In Globalize we have fall through cases that switch makes life easier, eg. https://github.com/jquery/globalize/blob/CLDR/src/datetime.js#L100 (ccc or cccc has the same output of EEE or EEEE)

PS: this code is under implementation, please don't consider possible errors.

tjvantoll commented 11 years ago

The new datepicker uses two switch statements, one of which uses a fall-through.

In general I don't like the idea of disallowing switchs as I find them more elegant even when fall-throughs are not used (for example key code handling throughout UI).

I'm in favor of a generic statement in the style guide. How about...

"The usage of switch statements is generally discouraged, but can be useful when there are a large number of cases - especially when multiple cases can be handled by the same block."

scottgonzalez commented 11 years ago

I'm good with something non-specific like that. @dmethvin ?

dmethvin commented 11 years ago

worksforme