Group multiple or statements within an IF in javascript

Question

The following code works:

$(".valClear").each(function () {
        if ($(this).val().indexOf(".png") == -1) {
            $(this).val('');
        }
    });

However this doesn't (the value is removed even if it contains .png or .jpeg), what am I doing wrong?

$(".valClear").each(function () {
        if (($(this).val().indexOf(".png") == -1) || ($(this).val().indexOf(".jpeg") == -1)) {
            $(this).val('');
        }
    });

@Heikki, looping over 10 fileuploads to see if they contain .png, .jpeg, .bmp. If they don't remove the current file. — The Muffin Man
– The Muffin Man, Commented Dec 25, 2010 at 23:51
@Dykam, this is enforced server side also, so no its not "So vulnerable in any way." — The Muffin Man
– The Muffin Man, Commented Dec 26, 2010 at 2:25

Nick Craver · Accepted Answer · 2010-12-26 00:03:14Z

3

You're doing an OR, seems like you want an AND, like this:

$(".valClear").each(function () {
    if (($(this).val().indexOf(".png") == -1) && ($(this).val().indexOf(".jpeg") == -1)) {
        $(this).val('');
    }
});

With your OR logic, the value will likely not have either one of them, and the other part will be true, you want an AND, meaning it has neither in the value.

You can also optimize it a bit, like this:

$(".valClear").each(function () {
  var val = $(this).val();
  if ((val.indexOf(".png") == -1) && (val.indexOf(".jpeg") == -1)) {
    $(this).val('');
  }
});

To stray quite a bit from the question, and towards your actual problem...I'd actually change up how you're doing this, using an array of allowed extensions and $.inArray() to check it, like this:

var allowedExtensions = ["png", "jpeg", "bmp"];
$(".valClear").each(function () {
  if ($.inArray($(this).val().split('.').pop(), allowedExtensions) == -1) {
    $(this).val('');
  }
});

You can test it here. You can see from the format, this is much easier to expand to allow additional extensions later.

edited Dec 26, 2010 at 0:03

answered Dec 25, 2010 at 23:44

Nick Craver

631k138 gold badges1.3k silver badges1.2k bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

Nick Craver Over a year ago

@nick - you're checking for -1 though, which means for this upload (the one you're looking over) you want to check it doesn't have .png and doesn't have .jpeg, in any other case clear the field. With an or it's always true, you want to check and make sure neither is true here...try it out, it'll give you the expected result.

The Muffin Man Over a year ago

@Nick Craver, this code keeps the .png but removes everything else that isn't a .png. It should be keeping the jpeg also

Nick Craver Over a year ago

@nick - are you sure you're testing .jpeg and not.jpg? Also make sure your code is exactly as I have above, you can see it working here: jsfiddle.net/nick_craver/Qvp9d

ifaour Over a year ago

@nick - be aware of the extension variety like ".jpg" too!!

Pointy Over a year ago

Regexes are starting to seem unfashionable to me around here, but I'd at least try a jsperf test to see if a regex against the value would be quicker than doing the array search.

|

Felix Kling · Accepted Answer · 2010-12-25 23:44:59Z

2

You have to use AND:

($(this).val().indexOf(".png") == -1) && ($(this).val().indexOf(".jpeg") == -1)

If the the string ends in .jpeg then the first expression will evaluate to true.
true OR false == true (the second expression won't even get evaluated).

Another option is to negate the whole expression (and test for whether .png and .jpeg are contained rather than not contained):

!($(this).val().indexOf(".png") > -1) || ($(this).val().indexOf(".jpeg") > -1))

answered Dec 25, 2010 at 23:44

Felix Kling

820k181 gold badges1.1k silver badges1.2k bronze badges

Comments

Pointy · Accepted Answer · 2010-12-25 23:46:25Z

1

Think of that second if test. It's like saying this:

If what you offer me for dinner is not roast chicken or it is not cold ham, then I'll have some

In that situation, you might get some cold ham or you might get roast chicken. Why? Because roast chicken is not cold ham, and because cold ham is not roast chicken. You said you're OK with whatever the kitchen has ready as long as what you get is either not roast chicken or not cold ham, so either of the two satisfies the condition.

If you phrase the qualifier with and instead of or, of course, then the kitchen will have to get creative with the left-over mashed potatoes.

answered Dec 25, 2010 at 23:46

Pointy

415k62 gold badges600 silver badges631 bronze badges

Comments

Heikki · Accepted Answer · 2010-12-26 00:07:03Z

1

Here's an alternative for and/or conditions. If the value doesn't match the regexp then..

$(".valClear").each(function () {
  if ( !$(this).val().match(/\.(png|jpeg)/) ) {
    $(this).val('');
  }
});

answered Dec 26, 2010 at 0:07

Heikki

15.4k2 gold badges57 silver badges49 bronze badges

Collectives™ on Stack Overflow

Group multiple or statements within an IF in javascript

4 Answers 4

11 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

11 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related