search

phetsims / pendulum-lab

"Pendulum Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 10 forks source link

Unfinished comment in PendulumLabRulerNode #62

Closed aaronsamuel137 closed 9 years ago

aaronsamuel137 commented 9 years ago

During code review #56, I noticed this code in PendulumLabRulerNode:

    RulerNode.call( this, rulerWidth, 34, tickWidth, rulerTicks, rulerUnitsString, { // -1px to

It's not clear to me what "// -1px to" means. Is this an unfinished comment? Can you either remove it or clarify what it means?

Also, it might be nice to make a var rulerHeight = 34, so it's more clear what that constant is doing there in the call to RulerNode.

pixelzoom commented 9 years ago

@aaronsamuel137 wrote:

Also, it might be nice to make a var rulerHeight = 34,

We might consider moving all of the constructor params to options. All of these parameters could have default values, and options are more "self documenting".

andrey-zelenkov commented 9 years ago

Remove outdated comment, move rulerHeight to constants block. Reassign to @aaronsamuel137 for verification.

aaronsamuel137 commented 9 years ago

Looks good. Closing