phetsims / color-vision

"Color Vision" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/color-vision
GNU General Public License v3.0
1 stars 7 forks source link

Some cases should use ternary operator #14

Closed samreid closed 10 years ago

samreid commented 10 years ago

It is a stylistic preference, but I recommend in cases like this:

    // create number of new photons proportional to intensity (between 1 and 5)
    var numToCreate;
    if ( intensity < cycleLength ) {
      numToCreate = 1;
    }
    else {
      numToCreate = Math.floor( 0.05 * intensity );
    }

To use the ternary operator like so:

      // create number of new photons proportional to intensity (between 1 and 5)
      var numToCreate = intensity < cycleLength ? 1 : Math.floor( 0.05 * intensity );

or

      // create number of new photons proportional to intensity (between 1 and 5)
      var numToCreate = intensity < cycleLength ? 
                        1 : 
                        Math.floor( 0.05 * intensity );
pixelzoom commented 10 years ago

I can live with it either way. I've seen style guides for other languages (C, C++) that recommend against the ternary operator because some feel that it's more difficult to read and a 'baroque' language feature.

If you do use the conditional (ternary) operator… With the single-line form, I prefer to put parentheses around the expression to improve readability. Especially if the expression involves more than one operator.

var numToCreate = ( intensity < cycleLength ) ? 1 : Math.floor( 0.05 * intensity );
aaronsamuel137 commented 10 years ago

I agree, the ternary operator looks a little nicer in this case, and I went with the parenthesis around the conditional expression. Assigning to @samreid for review

samreid commented 10 years ago

Looks great, closing.