From bb38ae3eec9daa0a17052642b133b9edc9ce4c6a Mon Sep 17 00:00:00 2001
From: AgustinVallejo <agustin.vallejo@udea.edu.co>
Date: Fri, 28 Feb 2025 14:38:41 -0500
Subject: [PATCH] Refactor documentation and remove unnecessary reentrant
 properties in angle properties
 https://github.com/phetsims/quantum-measurement/issues/109

---
 js/common/model/AbstractBlochSphere.ts            | 14 +++-----------
 js/common/model/AveragingCounterNumberProperty.ts |  6 ++++--
 js/common/view/ProbabilityValueControl.ts         |  4 +++-
 js/spin/model/SternGerlach.ts                     |  3 +--
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/js/common/model/AbstractBlochSphere.ts b/js/common/model/AbstractBlochSphere.ts
index 00833dcd..669dfcbe 100644
--- a/js/common/model/AbstractBlochSphere.ts
+++ b/js/common/model/AbstractBlochSphere.ts
@@ -23,12 +23,7 @@ export type AbstractBlochSphereOptions = SelfOptions & PickRequired<PhetioObject
 
 export default abstract class AbstractBlochSphere extends PhetioObject {
 
-  // REVIEW: Perhaps this is my lack of content understanding, but this documentation seems confusing. The sentence
-  // starts with "Angles" yet this is not an array. Do multiple angles determine the calculation of the azimuthal angle?
-  // Maybe clarify what knowledge a developer needs to have about azimuthal angles in implementation-notes.md and keep this
-  // documentation specific to the Property itself.
-  // Angles that determine the direction the vector representation is pointing at azimuthal angle, measured along the XY
-  // plane from the +X axis, goes from 0 to 2*PI in radians.
+  // Azimuthal angle, measured along the XY plane from the +X axis, goes from 0 to 2*PI in radians.
   public readonly azimuthalAngleProperty: NumberProperty;
 
   // Polar angle, measured from the +Z axis, goes from 0 to PI in radians.
@@ -48,19 +43,16 @@ export default abstract class AbstractBlochSphere extends PhetioObject {
 
     super( options );
 
-    // REVIEW: Document why the azimuthal and polar angles are reentrant and how we know that they will not hit an infinite loop
     this.azimuthalAngleProperty = new NumberProperty( options.initialAzimuthalAngle, {
       range: new Range( 0, 2 * Math.PI ),
       tandem: options.tandem.createTandem( 'azimuthalAngleProperty' ),
-      phetioReadOnly: true,
-      reentrant: true
+      phetioReadOnly: true
     } );
 
     this.polarAngleProperty = new NumberProperty( options.initialPolarAngle, {
       range: new Range( 0, Math.PI ),
       tandem: options.tandem.createTandem( 'polarAngleProperty' ),
-      phetioReadOnly: true,
-      reentrant: true
+      phetioReadOnly: true
     } );
 
     this.alphaProperty = new NumberProperty( 1, {
diff --git a/js/common/model/AveragingCounterNumberProperty.ts b/js/common/model/AveragingCounterNumberProperty.ts
index 2343c16d..a4a1b2b9 100644
--- a/js/common/model/AveragingCounterNumberProperty.ts
+++ b/js/common/model/AveragingCounterNumberProperty.ts
@@ -26,9 +26,11 @@ type DetectionCountSample = {
 
 class AveragingCounterNumberProperty extends NumberProperty {
 
-  // REVIEW: Some documentation for `totalAveragingPeriod` and `countSamplePeriod` would be helpful.
-  private readonly totalAveragingPeriod: number;
+  // Every countSamplePeriod we store a group of counts for that period
+  // Then, groups within a totalAveragingPeriod are averaged to get the final value
+  // So the displayed value updates every countSamplePeriod with the average of the last totalAveragingPeriod
   private readonly countSamplePeriod: number;
+  private readonly totalAveragingPeriod: number;
 
   // variables used in the detection rate calculation
   private currentDetectionCount = 0;
diff --git a/js/common/view/ProbabilityValueControl.ts b/js/common/view/ProbabilityValueControl.ts
index 9bc42c7a..d1d36a3c 100644
--- a/js/common/view/ProbabilityValueControl.ts
+++ b/js/common/view/ProbabilityValueControl.ts
@@ -39,7 +39,9 @@ export default class ProbabilityValueControl extends NumberControl {
 
     super( titleStringProperty, probabilityProperty, RANGE, optionize<ProbabilityValueControlOptions, SelfOptions, NumberControlOptions>()( {
 
-      // REVIEW: Add documentation describing why you need to create provide your own layoutFunction.
+      // Creating our own layout function because NumberControl doesn't have a native support for
+      //        title
+      //  < ------|------ >
       layoutFunction: ( titleNode, numberDisplay, slider, leftArrowButton, rightArrowButton ) => {
         assert && assert( leftArrowButton && rightArrowButton );
         return new VBox( {
diff --git a/js/spin/model/SternGerlach.ts b/js/spin/model/SternGerlach.ts
index 99adf740..ebdd9874 100644
--- a/js/spin/model/SternGerlach.ts
+++ b/js/spin/model/SternGerlach.ts
@@ -100,8 +100,7 @@ export default class SternGerlach extends PhetioObject {
     this.blockingModeProperty = new EnumerationProperty( BlockingMode.NO_BLOCKER, {
       tandem: tandem.createTandem( 'blockingModeProperty' ),
       phetioReadOnly: true,
-      phetioFeatured: true,
-      reentrant: true
+      phetioFeatured: true
     } );
 
     this.isDirectionControllableProperty = new BooleanProperty( false, {