Add code review comments, see: #109

js/common/model/AveragingCounterNumberProperty.ts

@@ -26,6 +26,7 @@ type DetectionCountSample = {
 
 class AveragingCounterNumberProperty extends NumberProperty {
 
+  // REVIEW: Some documentation for `totalAveragingPeriod` and `countSamplePeriod` would be helpful.
   private readonly totalAveragingPeriod: number;
   private readonly countSamplePeriod: number;
 
@@ -74,7 +75,7 @@ class AveragingCounterNumberProperty extends NumberProperty {
         count: this.currentDetectionCount
       } );
 
-      // Count the number of samples needed to reach the averaging period and total the counts that they contain.  Since
+      // Count the number of samples needed to reach the averaging period and total the counts that they contain. Since
       // the new samples are added to the end of the array, we need to start at the end and work backwards.
       let accumulatedSampleTime = 0;
       let accumulatedEventCount = 0;

js/photons/model/Laser.ts

@@ -67,11 +67,11 @@ class Laser extends PhetioObject {
   // The preset values of polarization direction that are available for the photons that are emitted.
   public readonly presetPolarizationDirectionProperty: Property<PolarizationPresets>;
 
-  // The custom polarization angle for the emitted photons.  This is only used when the preset direction is "custom".
+  // The custom polarization angle for the emitted photons. This is only used when the preset direction is "custom".
   public readonly customPolarizationAngleProperty: NumberProperty;
 
-  // The polarization angle of the emitted photons.  This is a derived - and thus read-only - property that is
-  // derived from the preset polarization direction and the custom polarization angle.  A value of null indicates that
+  // The polarization angle of the emitted photons. This is a derived - and thus read-only - Property that is
+  // derived from the preset polarization direction and the custom polarization angle. A value of null indicates that
   // the emitted photons are unpolarized, meaning that their individual polarization angles are random.
   public readonly polarizationAngleProperty: TReadOnlyProperty<number | null>;
 

js/photons/model/Photon.ts

@@ -24,6 +24,7 @@ export const RIGHT = new Vector2( 1, 0 );
 
 // Due to the experiment's nature, when photons are split,
 // the resulting states will either be measured as vertical or horizontal.
+// REVIEW: Unused
 export type PossiblePolarizationResult = 'vertical' | 'horizontal';
 
 class Photon {

js/photons/model/PhotonDetector.ts

@@ -40,7 +40,7 @@ export const RATE_RANGE = new Range( 0, 999 ); // in events per second
 
 class PhotonDetector extends PhetioObject implements TPhotonInteraction {
 
-  // The position of the detector in two-dimensional space.  Units are in meters.
+  // The position of the detector in two-dimensional space. Units are in meters.
   public readonly position: Vector2;
 
   // The direction in which the detector is looking for photons.
@@ -52,11 +52,11 @@ class PhotonDetector extends PhetioObject implements TPhotonInteraction {
   // detection aperture height, in meters
   public readonly apertureHeight = 0.05;
 
-  // A line in model space that represents the position of the detection aperture.  If a photon crosses this line, it
+  // A line in model space that represents the position of the detection aperture. If a photon crosses this line, it
   // will be detected but not absorbed.
   public readonly detectionLine: Line;
 
-  // The line in model space that represents the absorption line of the detector.  This is the line that the photon
+  // The line in model space that represents the absorption line of the detector. This is the line that the photon
   // must cross to be absorbed, at which point it disappears from the sim.
   public readonly absorptionLine: Line;
 
@@ -132,18 +132,18 @@ class PhotonDetector extends PhetioObject implements TPhotonInteraction {
         dt
       );
 
-      // Make sure the photon didn't cross both the detection and the absorption lines.  If this starts happening, the
+      // Make sure the photon won't cross both the detection and the absorption lines. If this starts happening, the
       // model may need to be extended to handle this case.
       assert && assert(
         !( detectionIntersectionPoint && absorptionIntersectionPoint ),
         'detection and absorption lines both crossed'
       );
 
-      // If the photon would cross the detection line, but not the absorption line, we'll consider it to be detected.
+      // If the photon will cross the detection line, but not the absorption line, we'll consider it to be detected.
       if ( detectionIntersectionPoint ) {
         mapOfStatesToInteractions.set( photonState, { interactionType: 'detectorReached', detectionInfo: { detector: this } } );
       }
-      // If the photon would cross the absorption line, but not the detection line, we'll consider it to be absorbed.
+      // If the photon will cross the absorption line, but not the detection line, we'll consider it to be absorbed.
       else if ( absorptionIntersectionPoint ) {
         mapOfStatesToInteractions.set( photonState, { interactionType: 'absorbed' } );
       }

js/photons/model/PhotonMotionState.ts

@@ -5,7 +5,7 @@
  * position, direction, and the probability of the photon being in that state.
  *
  * @author Agustín Vallejo
- * * @author John Blanco (PhET Interactive Simulations)
+ * @author John Blanco (PhET Interactive Simulations)
  */
 
 import Utils from '../../../../dot/js/Utils.js';
@@ -19,6 +19,8 @@ export class PhotonMotionState {
   public constructor( public position: Vector2,
                       public direction: Vector2,
                       public probability: number ) {
+    // REVIEW: Is this actually a no-op? It does create a class instance right? I might not understand the full
+    // behavior of an empty constructor.
     // no-op
   }
 

js/photons/model/PhotonsExperimentSceneModel.ts

@@ -57,13 +57,13 @@ class PhotonsExperimentSceneModel {
   public readonly verticalPolarizationDetector: PhotonDetector;
   public readonly horizontalPolarizationDetector: PhotonDetector;
 
-  // The normalized expectation value for the experiment.  This is essentially an average of all the possible outcomes
-  // of the experiment weighted by their likelihoods, normalized to a range of -1 to 1.  The value can also be null,
+  // The normalized expectation value for the experiment. This is essentially an average of all the possible outcomes
+  // of the experiment weighted by their likelihoods, normalized to a range of -1 to 1. The value can also be null,
   // which means the expectation value is not defined, which occurs when the photons are unpolarized.
   public readonly normalizedExpectationValueProperty: TReadOnlyProperty<number | null>;
 
-  // The normalized outcome value for the experiment.  This is the difference between the two measurements divided by
-  // the sum of the two measurements.  Its values range from -1 to 1.
+  // The normalized outcome value for the experiment. This is the difference between the two measurements divided by
+  // the sum of the two measurements. Its values range from -1 to 1.
   public readonly normalizedOutcomeValueProperty: TReadOnlyProperty<number>;
 
   // The photons that will be emitted and reflected in the experiment.
@@ -72,16 +72,16 @@ class PhotonsExperimentSceneModel {
   // Whether the simulation is currently playing, which in this case means whether the photons are moving.
   public readonly isPlayingProperty: BooleanProperty;
 
-  // Whether the photons behave as classical or quantum particles.  When they are classical, they will choose a path
-  // at the beam splitter.  When they are quantum, they will be in a superposition of states.
+  // Whether the photons behave as classical or quantum particles. When they are classical, they will choose a path
+  // at the beam splitter. When they are quantum, they will be in a superposition of states.
   public readonly particleBehaviorModeProperty: Property<SystemType>;
 
-  // Wether the probability accordion box is expanded
+  // Whether the probability accordion box is expanded
   public readonly isProbabilityAccordionExpandedProperty: BooleanProperty;
 
   public constructor( providedOptions: PhotonsExperimentSceneModelOptions ) {
 
-    // Create all photons that will be used in the experiment.  It works better for phet-io if these are created at
+    // Create all photons that will be used in the experiment. It works better for phet-io if these are created at
     // construction time and activated and deactivated as needed, rather than creating and destroying them.
     this.photonCollection = new PhotonCollection( providedOptions.tandem.createTandem( 'photonCollection' ) );
 

js/photons/model/PhotonsModel.ts

@@ -31,10 +31,10 @@ export const PhotonInteractionTypeValues = [
   // The photon was reflected by something, such as a mirror.
   'reflected',
 
-  // The photon was split into two possible state.
+  // The photon was split into two possible states.
   'split',
 
-  // The photon reached a detector.  If the photon is in a superposed state, it may or may not be detected, and the
+  // The photon reached a detector. If the photon is in a superposed state, it may or may not be detected, and the
   // client code will need to decide what to do.
   'detectorReached',
 
@@ -60,6 +60,7 @@ export type PhotonInteractionTestResult = {
   };
 };
 
+// REVIEW this is duplicated in Laser.ts
 export const ExperimentModeTypeValues = [ 'singlePhoton', 'manyPhotons' ] as const;
 export type ExperimentModeType = ( typeof ExperimentModeTypeValues )[number];