javascript style comparison not working

Question

I am working in JavaScript. I am facing an issue i.e. I want to place border around a div tag dynamically. My COde is Below:

function myfunction(var1) {
    if (document.getElementById(var1).style.border = "0px") {
        document.getElementById(var1).style.border = "1px solid green";
    } else {
        document.getElementById(var1).style.border = "0px";
    }
}

The code places border for first time but never removes it. i.e. else portion is not working. Any help would be appreciated.

@Dinesh that's jquery - do you see a jQuery tag? But Burhan you should cache your element, because you are doing a lot of unnecessary (but expensive!) searches in the dom-tree. Take a look at my answer on how to cache your element. — Christoph, Jan 09 '13 at 13:50
Well hope so it may help but i have done it with the courtesy of Lan Atkin :) Thanx for Your Good Response too .. PS: I have no experience of JQuery as well :( — Burhan Mughal, Jan 09 '13 at 14:18

Ian Atkin · Accepted Answer · 2013-01-09T14:04:24.397

Your statement is incorrect. It should be...

function myfunction(var1) {
    if(document.getElementById(var1).style.border == "0px") {
        document.getElementById(var1).style.border = "1px solid green";
    } else {
        document.getElementById(var1).style.border = "0px";
    }
}

In JavaScript = is the assignment operator. For comparisons you should use the equality operator, ==.

Your statement assigns 0px to the element and then stops. The else will never fire because the assignment evaluates to true every time.

Comparison Operator             Example    Result
==  Equal To                    x == y     false
!=  Not Equal To                x != y     true
<   Less Than                   x < y      true
>   Greater Than                x > y      false
<=  Less Than or Equal To       x <= y     true
>=  Greater Than or Equal To    x >= y     false

As mentioned in the comments below, the identity operator (===) would actually be better than the equality operator (==). The identity (===) operator behaves identically to the equality (==) operator except no type conversion is done, and the types must be the same to be considered equal. In this case a string is being compared with another string, so === could be used instead.

@Christoph Well, that's a different problem for next time! :) — Ian Atkin, Jan 09 '13 at 13:54
you should use === instead of == http://stackoverflow.com/questions/359494/javascript-vs-does-it-matter-which-equal-operator-i-use — Facundo Casco, Jan 09 '13 at 13:55
@F.C. Agreed. However... sometimes it's easier to just show the basic error first. Small steps... — Ian Atkin, Jan 09 '13 at 14:00
still missing the check for undefined. The first check will always go to the `else` branch while actually it (most likely) should go into the `if` branch. — Christoph, Jan 09 '13 at 14:05

score 2 · Answer 2 · answered Jan 09 '13 at 13:40

2

your if should contain logical operator:

if(document.getElementById(var1).style.border = "0px")

to

if(document.getElementById(var1).style.border == "0px")

answered Jan 09 '13 at 13:40

Alex Ackerman

1,341
12
14

Christoph · Answer 3 · 2013-01-09T13:45:55.203

Your if condition is always true:

style.border = "0px"

= assings your element the style border = 0px which is always true. You need to use the comparison operator == or even better the identity operator ===:

style.border == "0px"

also, you have to look if the property is set if the condition is checked for the first time, because style.border only recognizes inline-styles and thus is most likely not being set in the beginning.

The clean way is:

var el = document.getElementById(var1);
if(!el.style.border || el.style.border == "0px")
    el.style.border = "1px solid green";
} else {
    el.style.border = "0px";
}

score 0 · Answer 4 · answered Jan 09 '13 at 15:30

Everybody caught the assignment error, but the code still may not work sometimes.

The border property value, when you read it, is not necessarily what you wrote, but the browser's

interpretation of it- and shorthand assignments often inherit default properties for the missing specs.

The computer may not see the assignment to border as '0x', but as

'solid black 0px', or it may not exist except as individual properties-

no matter how you set it literally.

A more specific check with a closer look at the returned value is surer:

if(parseInt(document.getElementById(var1).style.borderTopWidth)){
    // the element has a non-zero border width
}

score -3 · Answer 5 · answered Jan 09 '13 at 13:40

You'd want to do

if(document.getElementById(var1).style.border == "0px") // note two = signs

Not

if(document.getElementById(var1).style.border = "0px")

And I'd have thought you'd want to set the border to 'none'

document.getElementById(var1).style.border = "none";

javascript style comparison not working

5 Answers5