search

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

issues with ThoughtBubble #45

Closed pixelzoom closed 10 years ago

pixelzoom commented 10 years ago

A couple of issues with ThoughtBubble...

  1. The derivation of colorProperty belongs in the model. Move the derived property into RGBModel, so that both model have a perceivedColorProperty. (Tip: Anytime you need to do instanceOf, it's likely that you have a problem with either type hierarchy structure or misplaced responsibilities.)
  2. Passing the entire model as a constructor parameter is unnecessarily broad. After doing 1, you'll be able to pass in just model.perceivedColorProperty, so the constructor will become:

function ThoughtBubble( colorProperty, yRadius, options )

aaronsamuel137 commented 10 years ago

I have made this change. Assigning to @pixelzoom for review

pixelzoom commented 10 years ago

Looks good, closing.