...

View Full Version : Simplify/compactify if-else statement



qwertyuiop
07-22-2010, 02:24 AM
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.

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!

rnd me
07-22-2010, 03:05 AM
discStatus[i] = {
"Open": "OPEN",
"Closed":"CLOSED",
"W-List":"WAITLISTED",
"Cancelled":"CANCELLED"
}[
sourceCode.substr(startStatus, 60).match(
/(Open|Closed|W\-List|Cancelled)/
)
] || "DEFAULT HERE";

qwertyuiop
07-22-2010, 03:26 AM
I'm getting the default case every time. :confused:

Old Pedant
07-22-2010, 03:54 AM
That's because match() isn't doing what RndMe expected.

Or what I expected, to be fair:


<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.

RandomUser531
07-22-2010, 04:03 AM
It's doing what it's supposed to do - just remove the parentheses and it won't return an array.

qwertyuiop
07-22-2010, 04:05 AM
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!

qwertyuiop
07-22-2010, 04:10 AM
What is this notation called btw? Looks like an object, but the square brackets throw me off

rnd me
07-22-2010, 10:58 AM
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:

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).

Old Pedant
07-22-2010, 07:52 PM
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:


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.

Old Pedant
07-22-2010, 07:55 PM
Or maybe:


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.)



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum