phetsims / trig-tour

"Trig Tour" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/trig-tour
GNU General Public License v3.0
2 stars 2 forks source link

Incorrect pattern for public function in UnitCircleView #55

Closed jessegreenberg closed 9 years ago

jessegreenberg commented 9 years ago

The function setLabelVisibility() is made public in the constructor of UnitCircleView and is called in other view files.

    //visibility of x,y, and '1' labels on xyR triangle
    this.setLabelVisibility = function( isVisible ) {
      positionLabels();
      labelCanvas.visible = isVisible;
    };

Is there a reason for this function being added publicly in the constructor? The function would adhere to PhET patterns by being added through inheritance.

jessegreenberg commented 9 years ago

Upon further inspection, I can see that keeping this.setLabelVisibility() in the constructor allows the function positionLabels to also be privately handled in the constructor.

I can now understand If you would like to keep setLabelsVisibility() in the constructor for this reason but documentation should be added to describe why the function is not in the inheritance block. Otherwise, both setLabelVisibility() and positionLabels() should be added to the inheritance block.

jessegreenberg commented 9 years ago

Seems cleanest to me to keep this.setLabelVisibility() public in the constructor so that positionLabels can remain private. I added a line of documentation explaining for future maintainers. Closing.