2

I have a form in which people can enter file paths. I want to make sure that the paths they are entering point to pictures so here is what I thought would work.

function checkExt()
{
    var extension= /* I know that the code to cut the extension off of the file
                      is working correctly so for now let's just go with it ok */
    if(extension!="jpg" || "gif" || "bmp" || "png" || "whatever else")
        alert("The file extension you have entered is not supported");
}

But this does not work. I have tracked it down to the if statement because if I select only 1 kind of file to check for, then it will work correctly. So my question to you is what the hell do I have to change to make this thing work correctly. I've been on this for about three hours now and it's driving me mad. Thanks for all of the help in advance.

ChapmIndustries
  • 1,401
  • 4
  • 17
  • 19

6 Answers6

6

That's a syntax and a logic error. It should be:

if (extension != "jpg" && 
    extension != "gif" && 
    extension != "bmp" && 
    extension != "png" && 
    extension != "whatever else") {
    // This will execute when the extension is NOT one of the expected 
    // extensions.
}

Furthermore, you could handle it a little more succinctly with a regular expression:

if (!/jpg|gif|bmp|png|whatever/.test(extension)) {
    // This will execute when the extension is NOT one of the expected 
    // extensions.
}

Addendum:

The examples above execute the body of the if-statement when the value of extension is not one of the supported values. If you wanted to execute the body of the if-statement when the value of extensions is one of the supported values, you would change the logic from not equal/and to equal/or, like so:

if (extension == "jpg" || 
    extension == "gif" || 
    extension == "bmp" || 
    extension == "png" || 
    extension == "whatever else") {
    // This will execute when the extension is one of the expected extensions.
}

And again, it'd be more concise using a regular expression:

// I just removed the leading ! from the test case.
if (/jpg|gif|bmp|png|whatever/.test(extension)) {
    // This will execute when the extension is one of the expected extensions.
}
FishBasketGordo
  • 22,904
  • 4
  • 58
  • 91
  • @j08691 - The logical error present would imply that `&&` should be used to make this comparison. – Travis J Aug 06 '12 at 21:22
  • @TravisJ - why are you including me in this? – j08691 Aug 06 '12 at 21:23
  • @j08691 - You removed your previous comment, "Don't you mean || instead of &&?" -j08691 – Travis J Aug 06 '12 at 21:24
  • @TravisJ - yeah I meant it for you, hence the removal from this answer. – j08691 Aug 06 '12 at 21:26
  • @j08691 - You perhaps need some rest, you seem very confused at the moment. Note that I had not commented prior to your statement. Moreover, your comment was directed at the person who posted this answer. – Travis J Aug 06 '12 at 21:29
  • Can someone explain why it would be AND instead of OR. It makes sense in my head that it is if extension=gif OR extension=jpg then do this. Wouldn't the AND imply that I want extension to equal all of those things? – ChapmIndustries Aug 06 '12 at 21:50
  • @ChapmIndustries You're not checking if the extension is equal to something. You're checking if it is *NOT* equal to something. You want to take action if the extension is *NOT* one of the supported extensions. Now, if you want to do something if the extension is one of the supported extensions, you would use equal/or. See my addendum. – FishBasketGordo Aug 07 '12 at 13:02
1

You should be using && instead of || and you must prefix the != operator with extension on each condition, not just the first one:

if (extension != "jpg" &&
    extension != "gif" &&
    extension != "bmp" && 
    extension != "png" &&
    extension != "whatever else")
jeff
  • 8,300
  • 2
  • 31
  • 43
1

As an alternative to the verbosity, use a regex:

if (extension.search(/jpg|gif|bmp|png/) === -1) {
    alert("The file extension you have entered is not supported.");
}
Brian Ustas
  • 62,713
  • 3
  • 28
  • 21
0

You need to include the parameters to compare in each clause.

if( extension != "jpg" || 
    extension != "gif" || 
    extension != "bmp" || 
    extension != "png" || 
    extension != "whatever else"){
 //TODO: handle condition
}

As others have noted, this is a logical error in that if "jpg" is the extension the condition will still be hit because "jpg" != "gif". You might want to consider using &&.

Travis J
  • 81,153
  • 41
  • 202
  • 273
0

You have to compare the variable extension for each tested extension. A compact way to do this would be:

var extension = whatever;
var supportedExtensions = ["jpg","gif","bmp","png",... whatever else];
if (supportedExtensions.indexOf(extension) < 0) alert("Unsupported extension");
Claudi
  • 5,224
  • 17
  • 30
0

The other answers have shown you how your javascript logic for multiple comparisons is wrong. But, a much better way to do this is to use a javascript map like this:

function checkExt() {
    var allowedExtensions = {
        "jpg": true, "gif": true, "bmp": true, "png": true, "whatever else":true
    };
    var extension= /* I know that the code to cut the extension off of the file
                      is working correctly so for now let's just go with it ok */
    if (allowedExtensions[extension] !== true) {
        alert("The file extension you have entered is not supported");
    }
}

The is a easier to maintain by just adding/removing items to/from the allowedExtensions object.

jfriend00
  • 683,504
  • 96
  • 985
  • 979