Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 10 of 10
  1. #1
    Regular Coder
    Join Date
    Jan 2004
    Location
    USA
    Posts
    364
    Thanks
    12
    Thanked 6 Times in 6 Posts

    Simplify/compactify if-else statement

    Hi guys,
    Is there a way to simplify/shorten this already small piece of code? Ternary is the only thing I can think of, but it will only complicate the code. I don't believe a switch statement would apply.
    Code:
    if (sourceCode.substr(startStatus, 60).indexOf("Open") !== -1){
    	discStatus[i] = "OPEN";
    }
    else if (sourceCode.substr(startStatus, 60).indexOf("Closed") !== -1){
    	discStatus[i] = "CLOSED";
    }
    else if (sourceCode.substr(startStatus, 60).indexOf("W-List") !== -1){
    	discStatus[i] = "WAITLISTED";
    }
    else if (sourceCode.substr(startStatus, 60).indexOf("Cancelled") !== -1){
    	discStatus[i] = "CANCELLED";
    }
    Basically, there's a small substring that I'm checking for one of the four values (open, closed, w-list, and cancelled).

    Thanks for your time!

  • #2
    Senior Coder rnd me's Avatar
    Join Date
    Jun 2007
    Location
    Urbana
    Posts
    4,461
    Thanks
    11
    Thanked 600 Times in 580 Posts
    Code:
    discStatus[i] = { 
     "Open": "OPEN",
     "Closed":"CLOSED",
     "W-List":"WAITLISTED",
     "Cancelled":"CANCELLED"
    }[ 
       sourceCode.substr(startStatus, 60).match(
            /(Open|Closed|W\-List|Cancelled)/
        )
    ] || "DEFAULT HERE";
    my site (updated 2014/10/20)
    BROWSER STATS [% share] (2014/9/03) IE7:0.1, IE8:4.3, IE11:9.2, IE9:2.7, IE10:2.6, FF:16.8, CH:47.5, SF:7.8, NON-MOUSE:37%

  • Users who have thanked rnd me for this post:

    qwertyuiop (07-22-2010)

  • #3
    Regular Coder
    Join Date
    Jan 2004
    Location
    USA
    Posts
    364
    Thanks
    12
    Thanked 6 Times in 6 Posts
    I'm getting the default case every time.

  • #4
    Supreme Master coder! Old Pedant's Avatar
    Join Date
    Feb 2009
    Posts
    27,700
    Thanks
    80
    Thanked 4,657 Times in 4,619 Posts
    That's because match() isn't doing what RndMe expected.

    Or what I expected, to be fair:
    Code:
    <script>
    
    var sourceCode = "lots and lots of text and more text and more text and finally we get to Open";
    var startStatus = 50;
    
    var s = sourceCode.substr(startStatus, 60);
    var m = s.match( /(Open|Closed|W\-List|Cancelled)/ );
    alert( m );
    </script>
    The alert shows
    Open,Open
    Back to the drawing board??

    Very peculiar.
    An optimist sees the glass as half full.
    A pessimist sees the glass as half empty.
    A realist drinks it no matter how much there is.

  • #5
    New Coder
    Join Date
    Jul 2010
    Posts
    61
    Thanks
    0
    Thanked 21 Times in 21 Posts
    It's doing what it's supposed to do - just remove the parentheses and it won't return an array.

  • Users who have thanked RandomUser531 for this post:

    qwertyuiop (07-22-2010)

  • #6
    Regular Coder
    Join Date
    Jan 2004
    Location
    USA
    Posts
    364
    Thanks
    12
    Thanked 6 Times in 6 Posts
    So a quick google search reveals that .match() returns an array with the strings that matched. If I'm understanding correctly, your example should only alert "Open", not "Open,Open"?

    EDIT: Thanks Phil removing the parentheses did the trick! And thanks to rnd_me!
    Last edited by qwertyuiop; 07-22-2010 at 05:08 AM.

  • #7
    Regular Coder
    Join Date
    Jan 2004
    Location
    USA
    Posts
    364
    Thanks
    12
    Thanked 6 Times in 6 Posts
    What is this notation called btw? Looks like an object, but the square brackets throw me off

  • #8
    Senior Coder rnd me's Avatar
    Join Date
    Jun 2007
    Location
    Urbana
    Posts
    4,461
    Thanks
    11
    Thanked 600 Times in 580 Posts
    Quote Originally Posted by qwertyuiop View Post
    So a quick google search reveals that .match() returns an array with the strings that matched. If I'm understanding correctly, your example should only alert "Open", not "Open,Open"?

    EDIT: Thanks Phil removing the parentheses did the trick! And thanks to rnd_me!
    when i hand-coded it, i left out the "g" flag on the regexp.
    parens act funny without it.

    without seeing the whole phrase, i wasn't sure how it would work out.

    a fail-safe pattern i often use is to default the match with an empty array and pluck the first element.

    changes in red:
    Code:
    discStatus[i] = { 
     "Open": "OPEN",
     "Closed":"CLOSED",
     "W-List":"WAITLISTED",
     "Cancelled":"CANCELLED"
    }[ 
    
    
     (sourceCode.substr(startStatus, 60).match(
            /(Open|Closed|W\-List|Cancelled)/g
        )||[])[0]
    
    ] || "DEFAULT HERE";
    this let's us pull [0] from each result, even it the .match() returns null.
    an invalid result will not be a valid object key, thus triggering the default.

    qwertyuiop:
    i don't know what this whole pattern is called, i just made it up as i read your question and looked at the logic.

    the first part is basically a "look-up table" as an object.
    the 2nd part i made up (hence the typo).
    the regexp extracts a key from the test string and the key looks-up the result.


    i don't usually use a regexp in conjunction with tables, but i use lookups to replace switches where i am comparing something stringy (one comparison against many equivalencies).
    Last edited by rnd me; 07-22-2010 at 12:09 PM.
    my site (updated 2014/10/20)
    BROWSER STATS [% share] (2014/9/03) IE7:0.1, IE8:4.3, IE11:9.2, IE9:2.7, IE10:2.6, FF:16.8, CH:47.5, SF:7.8, NON-MOUSE:37%

  • #9
    Supreme Master coder! Old Pedant's Avatar
    Join Date
    Feb 2009
    Posts
    27,700
    Thanks
    80
    Thanked 4,657 Times in 4,619 Posts
    Really fun stuff, RndMe. A nice answer, even if it is a bit "hacky"!!

    I do have to wonder if it's worth it in this case. Compared to, say:
    Code:
    var s = sourceCode.substr(startStatus, 60);
    var d = "";
    if      (s.indexOf("Open")      >= 0 ) d = "OPEN";
    else if (s.indexOf("Closed")    >= 0 ) d = "CLOSED";
    else if (s.indexOf("W-List")    >= 0 ) d = "WAITLISTED";
    else if (s.indexOf("Cancelled") >= 0 ) d = "CANCELLED";
    if ( d != "" ) discStatus[i] = d;
    It's about the same amount of code and the intent of the simpler form is much clearer. Could certainly see such a solution for more complex problems, though.
    An optimist sees the glass as half full.
    A pessimist sees the glass as half empty.
    A realist drinks it no matter how much there is.

  • #10
    Supreme Master coder! Old Pedant's Avatar
    Join Date
    Feb 2009
    Posts
    27,700
    Thanks
    80
    Thanked 4,657 Times in 4,619 Posts
    Or maybe:
    Code:
    var find = ["Open","Closed","W-List","Cancelled"];
    var answer = ["OPEN","CLOSED","WAITLISTED","CANCELLED"];
    var s = sourceCode.substr(startStatus, 60);
    for ( var f = 0; f < find.length; ++f )
    {
        if ( s.indexOf(find[f]) >= 0 ) discStatus[i] = answer[f];
    }
    (Though I'll readily admit that's a true hack.)
    An optimist sees the glass as half full.
    A pessimist sees the glass as half empty.
    A realist drinks it no matter how much there is.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •