Else statement not executed javascript

Question

Hey Guys in the below code every time the if statement is executed no matter what the condition is.

function addRow(tableID) {

            var table = document.getElementById(tableID);
            if((document.getElementById('select_degree').style.visibility = 'hidden')&&(document.getElementById('select_degree')!=null)){
              document.getElementById('select_degree').style.visibility = 'visible';
              document.getElementById('select_ratings').style.visibility  = 'visible';
             }

            else{ 
            var rowCount = table.rows.length;
            var new_row = table.rows[rowCount-1];  


            var row = table.insertRow(rowCount);


            var colCount = table.rows[0].cells.length;




            for(var i=0; i<colCount; i++) {

                var newcell = row.insertCell(i);


                newcell.innerHTML = new_row.cells[i].innerHTML;
            }
           }             
        }

When document.getelementById('add_degree') is null I get the error:

document.getElementById("select_degree") is null
addRow(tableID="add_degree")

But I already have a else statement if the element is null

If document.getElementById('select_degree') is null, then document.getElementById('select_degree').style.visibility will throw an error. If an error is thrown the whole script will terminate, the else branch will never be executed. Reverse the conditions. First check whether the element exists, and then assign (or compare) the other value. Does not make much sense to do it the other way round. — Felix Kling
– Felix Kling, Commented Feb 28, 2012 at 14:13
This is exactly why, when comparing against a literal, I make it a rule to have the literal as the LHS. — Ayush
– Ayush, Commented Feb 28, 2012 at 14:13

James Hill · Accepted Answer · 2012-02-28 14:33:20Z

2

Your if statement is not using the correct operator. Replace your = (used for assignment) with the comparison operator: ==. In addition, you must check if the element exists before attempting to check it's properties. If you don't, you'll get an exception when the element doesn't exist.

Here is your code, corrected and cleaned up:

function addRow(tableID) {
    var objSelectDegree = document.getElementById('select_degree');
    if (objSelectDegree != null && objSelectDegree.style.visibility == 'hidden') {
        objSelectDegree.style.visibility = 'visible';
        document.getElementById('select_ratings').style.visibility = 'visible';
    }
    else {
        // Moved table var to else block - it was not used unless else was hit
        var table = document.getElementById(tableID);
        var rowCount = table.rows.length;
        var new_row = table.rows[rowCount - 1];
        var row = table.insertRow(rowCount);
        var colCount = table.rows[0].cells.length;

        for (var i = 0; i < colCount; i++) {
            var newcell = row.insertCell(i);
            newcell.innerHTML = new_row.cells[i].innerHTML;
        }
    }
}

edited Feb 28, 2012 at 14:33

answered Feb 28, 2012 at 14:11

James Hill

62.1k22 gold badges150 silver badges166 bronze badges

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

2 Comments

Felix Kling Over a year ago

That does not solve the overall problem though (which is checking the conditions in the wrong order). Assigning a value in a condition is not wrong per se, though probably unintended.

James Hill Over a year ago

@FelixKling, you're right. I stopped looking once I saw the incorrect operator. Corrected.

Linus Kleen · Accepted Answer · 2012-02-28 14:39:47Z

2

This is why I love yoda conditionals. You're assigning a value (=) rather than evaluating it ==.

EDIT As suggested, I may point out that the order of checks in this particular statement is wrong. The check whether an element #select_degree exists should naturally occur before attempting to modify its properties.

However, given the element exists, the condition would still always evaluate true since the return value of the assignment (=) is used as an argument. As advocator for yoda conditionals, might I point out their advantage: for the following statement a syntax error would have been raised:

if ('hidden' = element.style.visibility)
{
// ...
}

edited Feb 28, 2012 at 14:39

answered Feb 28, 2012 at 14:12

Linus Kleen

34.8k10 gold badges97 silver badges100 bronze badges

2 Comments

Šime Vidas Over a year ago

Ah, that's why they switch the operands... Nice :)

Felix Kling Over a year ago

Please have a look at the comments I made on the question and other answers. Correcting this (if it is unintended at all) does not solve the overall problem.

Giel · Accepted Answer · 2012-02-28 14:21:13Z

1

in your if statement:

'=' should be '==' for starters

then:

(document.getElementById('select_degree').style.visibility == 'hidden')
&&
(document.getElementById('select_degree')!=null)

you check a property before checking if it is null, if you switch them around the program will check null first, and if it is, drop to else, and not check the second condition.

in this case, even if it is null, it is going to access .style.visibility

edited Feb 28, 2012 at 14:21

answered Feb 28, 2012 at 14:13

Giel

3971 silver badge9 bronze badges

Comments

papaiatis · Accepted Answer · 2012-02-28 14:11:57Z

0

style.visibility = 'hidden' -> style.visibility == 'hidden'

<CODE>
function addRow(tableID) {

   var table = document.getElementById(tableID);
   if((document.getElementById('select_degree').style.visibility == 'hidden') && (document.getElementById('select_degree')!=null)){
     document.getElementById('select_degree').style.visibility = 'visible';
     document.getElementById('select_ratings').style.visibility  = 'visible';
   }

   else{ 
     var rowCount = table.rows.length;
     var new_row = table.rows[rowCount-1];
     var row = table.insertRow(rowCount);                
     var colCount = table.rows[0].cells.length;

     for(var i=0; i<colCount; i++) {     
       var newcell = row.insertCell(i);                    
       newcell.innerHTML = new_row.cells[i].innerHTML;
     }
  }             
}

answered Feb 28, 2012 at 14:11

papaiatis

4,3114 gold badges28 silver badges39 bronze badges

1 Comment

Felix Kling Over a year ago

That does not solve the overall problem though (which is checking the conditions in the wrong order). Assigning a value in a condition is not wrong per se, though probably unintended.

Kunal Vashist · Accepted Answer · 2012-02-28 14:20:33Z

0

At first check the null condition, this is basic law if something is present then proceed further , as in your case if element is not present it will throw undefined and nothing will work. even replace = with == in hidden part

answered Feb 28, 2012 at 14:20

Kunal Vashist

2,4816 gold badges33 silver badges64 bronze badges

Collectives™ on Stack Overflow

Else statement not executed javascript

5 Answers 5

2 Comments

2 Comments

Comments

1 Comment

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

2 Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related