Why is this javascript if condition not working as expected?

Question

I am doing a comparison on dates in javascript. In this case date1 is empty "" and I can see the same in firebug. As per the code below, the first alert shouldn't be called because date1 == "", but for some reason the alert alert(" This is called...."); is invoked. What is wrong here?

if(date1 != null || date1 != ""){

        if( (date1 != null || date2 != "") && (date1 < date2)){
                alert(" This is called....");
                break;
        }
        else{
            alert(" That is called....");
            break;
        }

    }

The above if condition is inside a for loop, hence the break.

jfriend00 · Accepted Answer · 2012-06-17 17:56:34Z

2

I think you mean to be using && instead of || in your first comparision to make sure that both conditions are true and I think you have typo where you're using date1 instead of date2 in the second test. Further, you can just use if (date1) to simultaenously rule out null and "" and undefined and 0 and NaN and any other falsey value.

I think you want something like this:

if (date1) {
    if(date2 && date1 < date2) {
        alert(" This is called....");
    } else {
        alert(" That is called....");
    }
    break;
}

If what you're really trying to do is make sure that date1 and date2 are legal numbers, then I'd suggest you do this:

if (typeof date1 == "number") {
    if(typeof date2 == "number" && date1 < date2) {
        alert(" This is called....");
    } else {
        alert(" That is called....");
    }
    break;
}

Or, if they're supposed to be Date objects, then you can test for that:

if (date1 instanceof Date) {
    if(date2 instanceof Date && date1 < date2) {
        alert(" This is called....");
    } else {
        alert(" That is called....");
    }
    break;
}

edited Jun 17, 2012 at 17:56

answered Jun 17, 2012 at 17:32

jfriend00

711k104 gold badges1.1k silver badges1k bronze badges

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

2 Comments

yankee Over a year ago

Let's say that date1 is a unix timestamp. Then if date1 is set to zero it means 1970-1-1. And your check for if (date1) may surprise the user. Don't shortcut the check unless you really know what you are doing.

jfriend00 Over a year ago

@yankee - It is unclear what the type of the data is in date1 and date2. If the data is actually supposed to be numeric, I'd probably check to make sure it's numeric like if (typeof date1 == "number") rather than if (date1). I've added more info to my answer on that topic.

Jeremy Goodell · Accepted Answer · 2012-06-17 17:14:21Z

1

It looks correct to me. This statement evaluates to true because of the OR (||), since date1 is not null:

if(date1 != null || date1 != ""){

(it simplifies to if (true || false) { which is always true.

and the following statement evaluates to true as well because "" is not equal to null, and, presumably, date1 is less than date2:

if( (date1 != null || date2 != "") && (date1 < date2)){

answered Jun 17, 2012 at 17:14

Jeremy Goodell

19.1k5 gold badges37 silver badges54 bronze badges

4 Comments

Jeremy Goodell Over a year ago

Yeah, I guess someone else did -- seems pretty random to me. If I made a mistake, I'd love to know what it is, but a random downvote doesn't help anyone!!

yankee Over a year ago

@user918171: That would be a possibility, or you use and instead of or. Are you aware that || means OR and && means AND?

Jeremy Goodell Over a year ago

if (date1 != null && date1 != "")

Jeremy Goodell Over a year ago

or I think you could actually just say: if (date1) { but I'm not 100% sure about that.

Yurets · Accepted Answer · 2015-09-10 07:31:56Z

1

I want to suggest you this syntax

if(  (x==5) || (x==6)  || (x==7) || (x==8) )

this is best answer for checking OR condition using JavaScript and JQuery.

edited Sep 10, 2015 at 7:31

Yurets

4,00917 gold badges57 silver badges76 bronze badges

answered Sep 10, 2015 at 6:54

Vikas Kumar

211 bronze badge

1 Comment

Kushal Jayswal Over a year ago

In the past, I tried as below and it always went to the ELSE part: if(x == (5 || 6))

Tomasz Nurkiewicz · Accepted Answer · 2012-06-17 17:34:06Z

0

date1 is empty ""

Look at this condition:

date1 != null || date1 != ""

date1 is != null, the second term (date1 != "") isn't even evaluated because the first one passes - and the whole expressions evaluates to true.

Furthermore this condition:

date1 != null || date2 != ""

is also met (see above) - did you mean date2 != null this time? Nevertheless your This is called is displayed. BTW break has no sense in this context.

Finally you might consider simply:

if(date1) {

Not 1:1 equivalent, but pretty close.

edited Jun 17, 2012 at 17:34

answered Jun 17, 2012 at 17:27

Tomasz Nurkiewicz

342k72 gold badges713 silver badges680 bronze badges

Comments

yankee · Accepted Answer · 2012-06-17 17:48:22Z

Maybe that's a case where you should think about readability of your code. Here is my interpretation of your code:

if(!isBlank(date1){
    if(dateXIsSmallerThanDateY(date1, date2))
        alert(" This is called....");
    else
        alert(" That is called....");
}

function isBlank(date) {
    return date != null || date != "";
}

function dateXIsSmallerThanDateY(x,y) {
     return (x != null || y != "") && (x < y);
}

do you see a problem in dateXIsSmallerThanDateY? I can see one. What is that supposed to mean: x != null || y != ""? Probably it should read y != null || y != "", which in turn can be shortend to !isBlank(y).

Additionally the isBlank function does not check for beeing blank. Look what it does: It thinks that your date is blank if it is either not null or if it is not an empty string. Ok, maybe that is hard to read, so reverse this method:

function isNotBlank(date) {
    return date == null && date == "";
}

Positive comparisions are way easier to read. Do you see what is happening here? Can date be null and "" AT THE SAME TIME? No? Good. Than I guess you know what to do:

function isNotBlank(date) {
    return date != null && date != "";
}

Or in reverse:

function isBlank(date) {
    return date == null || date == "";
}

Collectives™ on Stack Overflow

Why is this javascript if condition not working as expected?

5 Answers 5

2 Comments

4 Comments

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

4 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related