How NOT to Code - Challenge 10

Jan 28, 2010

Today's challenge is in javascript. Can you find all the problems with this code? On some browsers you may see a double greater sign on the age and gpa conditionals. You can simply view that as a single greater than sign. Sorry I think this a small issue in encoding via this blog software.


<script type="text/javascript">
   var gpa = 3.92;
   var year = "4";
   var age = 22;
   var id = "123";

   if (id == "123"){
      if (age < 21){
         if (year = 4){
            if (gpa > 3.5){
               if (id === 123){
                  alert('good student');
               }else{
                  alert('bad student');
               }
            }
         }
      }
   }
</script>

Comments

Amy

Amy wrote on 01/28/105:25 PM

The "double greater than" looks like a bitwise shift. Just sayin.

Where to start?

First, there doesn't seem to be much rhyme or reason as to why some things are strings and some numbers.

if (year=4) not only sets the year to 4 (instead of the original "4"), but it will always return true. This is an easy mistake to make--I'm sure we've all made it at one time or another.

The second check on id is not only unneccessary (since we already checked it), but it will always return false, as the strict equality check will see '123' (string) as !== 123 (number).

There's also no consistency about using single or double quotes, which isn't "incorrect," but just feels 'messy' :).

Also, I didn't see the spec for this, but it seems like you might also want to alert for GPA < 3.5 and students who are under 22 and in different years (didn't respond to all possible conditions).
Kyle

Kyle wrote on 01/28/105:57 PM

One of the tricks I learned back in college to make sure that I didn't accidentally assign values to variables when I was comparing them is to put the constant on the left side of the comparison.

So, instead of if( year = 4 ) executing correctly, 'if( 4 = year )' would throw an error (or at least, it should!).

It doesn't really matter for < or > comparisons, but it's a good habit to get into anyway.
Larry Battle

Larry Battle wrote on 01/29/1012:09 AM

The main problem is size, speed and security.
Too many if and else, var and global variables.
Also, I think you meant age>21 == "bad student".

<code><pre>
var gpa = 3.92,year = 4,age = 22,id = <span class='cc_value'>"123"</span>, statusOfStudent = (id != <span class='cc_value'>"123"</span> || year != 4 || age > 21 || gpa < 3.5) ? 'bad student': 'good student';

window.alert( statusOfStudent );
</pre></code>

double post...

var gpa = 3.92,year = 4,age = 22,id = &quot;123&quot;, statusOfStudent = (id != &quot;123&quot; || year != 4 || age &lt; 21 || gpa &gt; 3.5) ? 'bad student': 'good student';

window.alert( statusOfStudent );
John Mason

John Mason wrote on 02/09/108:57 AM

Yep, I think you guys got most of the issues. The example is a bit silly but just showing the three main uses of the equals sign in javascript.

Single Equals sign is for assignment

Double Equals sign is for a untyped conditional check

Triple Equals sign is for a typed conditional check which is very rarely used but can be very handy.

Write your comment



(it will not be displayed)