refactor a section of createVertexArea to reduce duplication, complex…

…ity, and complete review comment, see #398
phetsims · Mar 21, 2023 · f6b47f5 · f6b47f5
1 parent 89a853b
commit f6b47f5
Showing 1 changed file with 35 additions and 40 deletions.
diff --git a/js/quadrilateral/model/QuadrilateralUtils.ts b/js/quadrilateral/model/QuadrilateralUtils.ts
@@ -161,50 +161,15 @@ export default class QuadrilateralUtils {
       secondRay = new Ray2( vertex2.positionProperty.value, secondRayDirection );
     }
 
-    // REVIEW: There is some duplication and complexity in the following part. Can it be simplified?
+    // Find the intersection points between the calculated rays and directed boundary lines.
     let firstRayIntersectionLinePair: null | LineIntersectionPair = null;
     let secondRayIntersectionLinePair: null | LineIntersectionPair = null;
-
     directedLines.forEach( line => {
-      const firstLineIntersections = line.intersection( firstRay! );
-      const secondLineIntersections = line.intersection( secondRay! );
-
-      if ( firstLineIntersections.length > 0 ) {
-        firstRayIntersectionLinePair = {
-          line: line,
-          intersectionPoint: firstLineIntersections[ 0 ].point
-        };
-      }
-      else {
-
-        // There wasn't an intersection, the ray intersected exactly with a corner of the bounds, which is not
-        // a defined intersection according to Kite.
-        const intersectionPoint = QuadrilateralUtils.getLinePositionAlongRay( firstRay!, line );
-        if ( intersectionPoint ) {
-          firstRayIntersectionLinePair = {
-            line: line,
-            intersectionPoint: intersectionPoint
-          };
-        }
+      if ( firstRayIntersectionLinePair === null ) {
+        firstRayIntersectionLinePair = QuadrilateralUtils.getLineIntersectionPair( firstRay!, line );
       }
-
-      if ( secondLineIntersections.length > 0 ) {
-        secondRayIntersectionLinePair = {
-          line: line,
-          intersectionPoint: secondLineIntersections[ 0 ].point
-        };
-      }
-      else {
-
-        // There wasn't an intersection, the ray intersected exactly with a corner of the bounds, which is not
-        // a defined intersection according to Kite.
-        const intersectionPoint = QuadrilateralUtils.getLinePositionAlongRay( secondRay!, line );
-        if ( intersectionPoint ) {
-          secondRayIntersectionLinePair = {
-            line: line,
-            intersectionPoint: intersectionPoint
-          };
-        }
+      if ( secondRayIntersectionLinePair === null ) {
+        secondRayIntersectionLinePair = QuadrilateralUtils.getLineIntersectionPair( secondRay!, line );
       }
     } );
     assert && assert( firstRayIntersectionLinePair && secondRayIntersectionLinePair, 'ray intersections were not found' );
@@ -249,6 +214,36 @@ export default class QuadrilateralUtils {
     return shape;
   }
 
+  /**
+   * Get the LineIntersectionPair from the intersection between the ray and the Line. Handles a special case where
+   * the ray intersects the exact start/end of a Line. If there is no intersection point, returns null.
+   */
+  private static getLineIntersectionPair( ray: Ray2, line: Line ): LineIntersectionPair | null {
+    let intersectionLinePair: null | LineIntersectionPair = null;
+
+    const rayIntersections = line.intersection( ray );
+    if ( rayIntersections.length > 0 ) {
+      intersectionLinePair = {
+        line: line,
+        intersectionPoint: rayIntersections[ 0 ].point
+      };
+    }
+    else {
+
+      // There wasn't an intersection, the ray intersected exactly with a corner of the bounds, which is not
+      // a defined intersection according to Kite.
+      const intersectionPoint = QuadrilateralUtils.getLinePositionAlongRay( ray, line );
+      if ( intersectionPoint ) {
+        intersectionLinePair = {
+          line: line,
+          intersectionPoint: intersectionPoint
+        };
+      }
+    }
+
+    return intersectionLinePair;
+  }
+
   /**
    * Returns true if the provided point lies on the ray.
    */