javascript - Is there a better way to write this mutiple or conditional? - Stack Overflow

admin管理员组

文章数量:1345138

I have the following IF statement in javascript:

if ( !(cmd === 'JustifyLeft' || cmd === 'JustifyRight' || cmd === 'JustifyCenter' || cmd === 'JustifyFull') )

Any suggestions on how it could be written in a cleaner way?

Thanks

I have the following IF statement in javascript:

if ( !(cmd === 'JustifyLeft' || cmd === 'JustifyRight' || cmd === 'JustifyCenter' || cmd === 'JustifyFull') )

Any suggestions on how it could be written in a cleaner way?

Thanks

• javascript
• operators
• conditional-statements

Share Improve this question Follow edited Nov 27, 2013 at 10:07 Relequestual 12.4k77 gold badges5151 silver badges8787 bronze badges asked Jun 9, 2009 at 7:25 pokemonpokemon 3

• 3 Better how? To make it . . . Faster? Clearer? Less code? More Generic? More Specific? What do you think is wrong with it? – Binary Worrier Commented Jun 9, 2009 at 7:35
• The if-statement looks just fine to me. Perhaps some line-break formatting would improve readability. – cllpse Commented Jun 9, 2009 at 8:04
• I think this question is more appropriate for codereview.stackexchange. – Phil Helix Commented Mar 12, 2011 at 10:57

Add a ment  | 

4 Answers 4

Sorted by: Reset to default 17

if(!cmd.match(/^Justify(Left|Right|Center|Full)$/))

In response to a few ments you can also mimic your strict parison with a small edit:

if( typeof cmd != 'String' || !cmd.match(/^Justify(Left|Right|Center|Full)$/))

This will react in the exact same way as your current code, ignoring anything that's not a string.

Personally I think it is highly unlikely that you will need it.

This sounds like a good situation to use a switch. Just be aware that switches only do equality checking (==) not identity checking (===), though this should be fine.

switch (cmd) {
    case "JustifyLeft" :
    case "JustifyRight" :
    case "JustifyCenter" :
    case "JustifyFull" :
        // do something
    break;
    case "somethingElse" :
    default:
        // do something else
    break;
}

I would create a IsJustifyCommand(s) method or create a mand abstract class that has a IsJustifyCommand() method on it. Then the code will read like a description of what it is trying to do.

Using regex may be neat, but will lead to maintenance problems if someone that is not a hard-core JavaScript programmer has to work on the code. However if you have lots of cases when regex is a good solution, then use it, as anyone looking at the code will soon pick it up.

(However I am a C# programmer not a JavaScript programmer, but like most programmer I have to look at / edit JavaScript code sometimes. I think most JavaScript is maintained by none JavaScript programmers.)

I hate when something is written like that. First I look at the code and think "if cmd is equal to JustifyLeft or JustifyRight... then invert that and... if that's true do the thing.. so that means if it's JustifyLeft...". For me it takes alot of time and I have to re-read the line to be sure I got it right.

I think it's better to write.

if ((cmd !== 'JustifyLeft') && (cmd !== 'JustifyRight') && (cmd !== 'JustifyCenter') && (cmd !== 'JustifyFull'))

It might be a little more verbose but I find it easier to follow. I read it as "cmd can't be any of the Justify-strings". Checking a long boolean expression and then inverting the whole answer is irritating.

I like the solution from scragar, just wanted to give my thoughts about inverting a long boolean expression.

本文标签： javascriptIs there a better way to write this mutiple or conditionalStack Overflow