handle review comments, #165

js/chart-intro/model/ChartIntroModel.ts

@@ -111,18 +111,15 @@ class ChartIntroModel extends BANModel<ParticleNucleus> {
   public override step( dt: number ): void {
     super.step( dt );
 
-    // step the miniParticleAtom nucleons
-    // REVIEW - code duplication - the same closure is passed to forEach loops below.
-    this.miniParticleAtom.protons.forEach( particle => {
-      assert && assert( !this.outgoingParticles.includes( particle ), 'should not double step particle' );
-      assert && assert( !this.particles.includes( particle ), 'should not double step particle' );
-      particle.step( dt );
-    } );
-    this.miniParticleAtom.neutrons.forEach( particle => {
+    const stepParticle = ( particle: Particle ) => {
       assert && assert( !this.outgoingParticles.includes( particle ), 'should not double step particle' );
       assert && assert( !this.particles.includes( particle ), 'should not double step particle' );
       particle.step( dt );
-    } );
+    };
+
+    // step the miniParticleAtom nucleons
+    this.miniParticleAtom.protons.forEach( stepParticle );
+    this.miniParticleAtom.neutrons.forEach( stepParticle );
 
     // When decaying, the animated particle from the miniParticleAtom is removed from it, but still needs to be stepped off the screen
     this.outgoingParticles.forEach( particle => {

js/common/BANColors.ts

@@ -1,10 +1,9 @@
 // Copyright 2021-2023, University of Colorado Boulder
 
-// REVIEW: Does the author attribution need to be updated?  Looks like a cut-and-paste error to me.
 /**
  * BANColors defines the color profile for this sim.
  *
- * @author Chris Malley (PixelZoom, Inc.)
+ * @author Luisa Vargas
  */
 
 import { Color, ProfileColorProperty } from '../../../scenery/js/imports.js';

js/common/model/AlphaParticle.ts

@@ -44,10 +44,8 @@ class AlphaParticle extends ParticleAtom {
       easing: Easing.LINEAR
     } );
 
-    // remove all particles individually from the model
-    // REVIEW: I (jbphet) ran a test where I put a debug statement in this listener and then hit Reset All before the
-    //         animation was complete, and the listener never fired, so the dispose function was never called.  This
-    //         might lead to a minor memory lead.  Perhaps endedEmitter should be used instead?
+    // Remove all particles individually from the model. If reset or interrupt occurs, there is no need to call this
+    // because these particles are cleaned up via other means.
     alphaParticleEmissionAnimation.finishEmitter.addListener( () => {
       this.neutrons.forEach( neutron => {
         removeParticle( neutron );