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

naming convention for imported images #47

Closed pixelzoom closed 10 years ago

pixelzoom commented 10 years ago

Your naming convention for imported images confused me a little when first reading your code. Here's an example from FlashlightNode:

19 var flashlight = require( 'image!COLOR_VISION/flashlight.png' ); 35 var flashlightImage = new Image( flashlight, { scale: SCALE } );

While reading the code, I expected flashlight to be a model element, not an imported image. I recommend naming imported images with 'Image' suffix, and nodes with 'Node'. So the above would become:

19 var flashlightImage = require( 'image!COLOR_VISION/flashlight.png' ); 35 var flashlightNode = new Image( flashlightImage, { scale: SCALE } );

Before you proceed with changing this through your code, you may want to see how @samreid feels about it. He may have a different opinion.

aaronsamuel137 commented 10 years ago

I changed names to match this convention. There are still cases with icons where I have left the Icon suffix instead of Image, as in:

  var whiteLightIcon = require( 'image!COLOR_VISION/white-light-icon.png' );
  var singleColorLightIcon = require( 'image!COLOR_VISION/single-color-light-icon.png' );
  var beamViewIcon = require( 'image!COLOR_VISION/beam-view-icon.png' );
  var photonViewIcon = require( 'image!COLOR_VISION/photon-view-icon.png' );

I am fine to change these to Image too, just want to be sure what the convention is. Any thoughts?

samreid commented 10 years ago

In my opinion, it is ok to use "icon" as the suffix for images used as icons.

aaronsamuel137 commented 10 years ago

Assigning to @pixelzoom for review.

pixelzoom commented 10 years ago

Looks good. "Icon", "image" or anything else to differentiate them from model elements is fine. Closing.