CODEWITHSUNDEEP
javascriptjqueryif-statement
d3c0y
d3c0y

Reputation: 984

Javascript if else statement with multiple or following condition

I'm detecting extensions then taking actions based on the extension. So my question is why doesn't this work, seems logical enough eh?

var ext = url.split('.').pop().toLowerCase();
if (ext == 'avi' || 'mpg' || 'mpeg' || 'mp4' || '3gp') {
  This is a video (this always returns true...?)
} else if (ext == 'jpg' || 'jpeg' || 'gif' || 'png' || 'bmp') {
  This is a picture
} else {
  This extension isn't supported here
}

But this does? Unnecessary overhead?

var ext = url.split('.').pop().toLowerCase();
if (ext == 'avi' || ext == 'mpg' || ext == 'mpeg' || ext == 'mp4') {
  This is a video
} else if (ext == 'jpg' || ext == 'jpeg' || ext == 'gif' || ext == 'png') {
  This is a picture
} else {
  This extension isn't supported here
}

Is there a syntax issue I'm missing to make this work like example 1 without hitting the variable over and over? Concerned because this list is a lot larger than what is pictured in regards to the amount of extensions and seems like a lot of unnecessary code when it's all said and done.

Upvotes: 0

Views: 176

Answers (3)

megawac
megawac

Reputation: 11373

The line ext == 'avi' || 'mpg' || 'mpeg' || 'mp4' || '3gp' will always be true as you are comparing if ext is avi or if any of 'mpg' || 'mpeg' || 'mp4' || '3gp' are truthy.

The == operator only compares a single variable for future reference.

Another way you can write this comparison with a switch is as follows:

switch(ext) {//switch with fall throughs
    case 'avi':
    case 'mpg':
    case 'mpeg':
    case 'mp4':
        //we got a video
        break;
    case 'jpg':
    case 'jpeg':
    case 'gif':
    case 'png':
        //its a picture
        break;
    default:
        //this extension isn't suupported
}

Upvotes: 3

nnnnnn
nnnnnn

Reputation: 150080

"So my question is why doesn't this work"

Because that's just not what the || operator does.

The shortest syntax I can think of to implement your concept is to use a regex test for each condition:

if (/^(avi|mpg|mpeg|mp4|3gp)$/.test(ext)) {

Or you can use an array:

if (['avi', 'mpg', 'mpeg', 'mp4', '3gp'].indexOf(ext) != -1) {

(Assuming you're not worried about IE<=8, or are using a shim, or use jQuery's $.inArray() instead of .indexOf().)

Or this seems an obvious place to use a switch statement:

var ext = url.split('.').pop().toLowerCase();
switch(ext) {
    case 'avi':
    case 'mpg':
    case 'mpeg':
    case 'mp4':
    case '3gp':
       //  This is a video (this always returns true...?)
       break;
    case 'jpg':
    case 'jpeg':
    case 'gif':
    case 'png':
    case 'bmp':
       // This is a picture
       break;
    default:
       //  This extension isn't supported here
       break;
}

Upvotes: 1

Arun P Johny
Arun P Johny

Reputation: 388436

You first if condition is always truthy.

If you have a lot of values to check then I would suggest something like

var video = ['avi', 'mpg'];
var audio = ['mpg', 'mpeg'];
if($.inArray(ext, video)){
    //video
} if($.inArray(ext, audio)){
    //audio
} else {
}

Upvotes: 1

Related Questions