Addressed REVIEWS: Updated doc for permanent links.

phetsims/masses-and-springs-basics#48
phetsims · Feb 1, 2019 · 67ad3a1 · 67ad3a1
1 parent c2cc4d0
commit 67ad3a1
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 9 deletions.
diff --git a/js/common/view/EnergyGraphNode.js b/js/common/view/EnergyGraphNode.js
@@ -112,8 +112,7 @@ define( function( require ) {
       scale: 0.7
     } );
 
-    // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are
-    // REVIEW: created.
+    // Link exists for sim duration. No need to unlink.
     model.firstSpring.thermalEnergyProperty.link( function( value ) {
       clearThermalButton.enabled = ( value > 0 );
       clearThermalButton.pickable = ( value > 0 );

diff --git a/js/common/view/GravityAndDampingControlNode.js b/js/common/view/GravityAndDampingControlNode.js
@@ -253,9 +253,7 @@ define( function( require ) {
       gravityComboBox.top = gravityNumberControl.bottom + 10;
     }
 
-    // Responsible for managing bodies
-    // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are
-    // REVIEW: created.
+    // Responsible for managing bodies. Link exists for sim duration. No need to unlink.
     model.bodyProperty.link( function( newBody, oldBody ) {
       var body = _.find( Body.BODIES, newBody );
 

diff --git a/js/common/view/MassNode.js b/js/common/view/MassNode.js
@@ -78,9 +78,7 @@ define( function( require ) {
         return modelBounds;
       } );
 
-    // Update the size of the massNode
-    // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are
-    // REVIEW: created for a single mass
+    // Update the size of the massNode. Link exists for sim duration. No need to unlink.
     mass.radiusProperty.link( function( radiusValue ) {
 
       self.rect.rectBounds = new Bounds2(

diff --git a/js/common/view/OscillatingSpringNode.js b/js/common/view/OscillatingSpringNode.js
@@ -73,17 +73,19 @@ define( function( require ) {
       self.y = modelViewTransform2.modelToViewY( spring.positionProperty.get().y - spring.lengthProperty.get() );
     }
 
-    // REVIEW: Probably worth noting that this type creates permanent listeners.
+    // Link exists for sim duration. No need to unlink.
     spring.naturalRestingLengthProperty.link( function( springLength ) {
       self.loopsProperty.set( MAP_NUMBER_OF_LOOPS( springLength ) );
       updateViewLength();
     } );
 
+    // Link exists for sim duration. No need to unlink.
     spring.lengthProperty.link( function() {
       updateViewLength();
     } );
 
     // ParametricSpringNode width update. SpringConstant determines lineWidth
+    // Link exists for sim duration. No need to unlink.
     spring.thicknessProperty.link( function( thickness ) {
       self.lineWidthProperty.set( thickness );
     } );