CodFusion

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 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 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 );