Simplify evaluate call sites in Electron.ts

[samreid]

Discovered in #103 and related to #137, we observed code in Electron.ts:

  // Electron's position, relative to the coil's position. This value is changed internally and should not be changed by
  // clients. Also be aware that the entire Vector2 instance may be changed (rather than just the x and y values changing).
  public readonly position: Vector2;

 // Reusable Vector2 instances
  private readonly reusablePosition1: Vector2;
  private readonly reusablePosition2: Vector2;

      const returnValue = ( this.position === this.reusablePosition1 ) ? this.reusablePosition2 : this.reusablePosition1;
      this.position = coilSegment.evaluate( this.coilSegmentPosition, returnValue );

Since the comment in #137 was incomplete, it is difficult to know why this strategy was chosen. But it is unnecessary. By introducing evaluateX and evaluateY methods, we can simplify the implementation like so:

Subject: [PATCH] Add documentation, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/103
---
Index: js/common/model/Electron.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Electron.ts b/js/common/model/Electron.ts
--- a/js/common/model/Electron.ts	(revision aca4a2b1889aa9c4688845388e3831034f04a917)
+++ b/js/common/model/Electron.ts	(date 1711567930286)
@@ -56,7 +56,7 @@
 
   // Electron's position, relative to the coil's position. This value is changed internally and should not be changed by
   // clients. Also be aware that the entire Vector2 instance may be changed (rather than just the x and y values changing).
-  public position: Vector2;
+  public readonly position: Vector2;
 
   // Ordered collection of the segments that make up the coil
   private readonly coilSegments: CoilSegment[];
@@ -74,10 +74,6 @@
   // Scale for adjusting speed.
   private readonly speedScaleProperty: TReadOnlyProperty<number>;
 
-  // Reusable Vector2 instances
-  private readonly reusablePosition1: Vector2;
-  private readonly reusablePosition2: Vector2;
-
   public constructor( currentAmplitudeProperty: TReadOnlyProperty<number>, currentAmplitudeRange: Range, providedOptions: ElectronOptions ) {
 
     const options = providedOptions;
@@ -86,12 +82,12 @@
     this.currentAmplitudeProperty = currentAmplitudeProperty;
     this.currentAmplitudeRange = currentAmplitudeRange;
 
-    this.reusablePosition1 = new Vector2( 0, 0 );
-    this.reusablePosition2 = new Vector2( 0, 0 );
-
     const coilSegment = options.coilSegments[ options.coilSegmentIndex ];
 
-    this.position = coilSegment.evaluate( options.coilSegmentPosition, this.reusablePosition1 );
+    this.position = new Vector2(
+      coilSegment.evaluateX( options.coilSegmentPosition ),
+      coilSegment.evaluateY( options.coilSegmentPosition )
+    );
     this.coilSegments = options.coilSegments;
     this.coilSegmentIndex = options.coilSegmentIndex;
     this.coilSegmentPosition = options.coilSegmentPosition;
@@ -153,8 +149,8 @@
       // Evaluate the quadratic to determine the electron's position relative to the segment.
       // Use a different reusable Vector2 each time that curve.evaluate is called, so that
       const coilSegment = this.coilSegments[ this.coilSegmentIndex ];
-      const returnValue = ( this.position === this.reusablePosition1 ) ? this.reusablePosition2 : this.reusablePosition1;
-      this.position = coilSegment.evaluate( this.coilSegmentPosition, returnValue );
+      this.position.x = coilSegment.evaluateX( this.coilSegmentPosition );
+      this.position.y = coilSegment.evaluateY( this.coilSegmentPosition );
     }
   }
 

This has the following benefits:

• Simpler implementation
• No swapping reusablePosition vectors
• position can be marked as readonly
• Faster startup time since Electron isn't allocating extra Vector2 instances
• Lower memory overhead since Electron isn't allocating extra Vector2 instances

If we adopt this approach, it will be necessary to add evaluateX and evaluateY in CoilSegment and QuadraticBezierSpline, but that will be straightforward since the dimensions are independent.

[pixelzoom]

There's no need to modify evaluate, see 7006afe. I've also improved the Electron API by making position private, and adding getters for x and y, which prevents clients from mutating position.

@samreid please review, close if OK.

[samreid]

That's even better! Closing.