From c0af6b5a8804d6bfb953f55223ed8deae76cde86 Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Fri, 3 Mar 2023 14:39:12 -0700 Subject: [PATCH] Support array or element, and update REVIEW comments for query parameters, see https://github.com/phetsims/quadrilateral/issues/398 --- .../QuadrilateralQueryParameters.ts | 3 +- .../model/QuadrilateralShapeModel.ts | 34 ++++++++++--------- js/quadrilateral/view/QuadrilateralAlerter.ts | 2 +- js/quadrilateral/view/SideDescriber.ts | 2 +- js/quadrilateral/view/VertexDescriber.ts | 2 +- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/js/quadrilateral/QuadrilateralQueryParameters.ts b/js/quadrilateral/QuadrilateralQueryParameters.ts index 1c5e7179..6b4417e5 100644 --- a/js/quadrilateral/QuadrilateralQueryParameters.ts +++ b/js/quadrilateral/QuadrilateralQueryParameters.ts @@ -9,6 +9,7 @@ import quadrilateral from '../quadrilateral.js'; import { SoundDesign } from './model/QuadrilateralSoundOptionsModel.js'; +// REVIEW: Which of these should be marked as public? const QuadrilateralQueryParameters = QueryStringMachine.getAll( { // The tolerance interval for the angle calculations which determine when sides opposite sides are parallel. @@ -118,8 +119,6 @@ const QuadrilateralQueryParameters = QueryStringMachine.getAll( { // QuadrilateralSoundOptionsModel.SoundDesign as a string. See https://github.com/phetsims/quadrilateral/blob/master/js/quadrilateral/model/QuadrilateralSoundOptionsModel.ts#L37-L53 soundDesign: { type: 'string', - - // REVIEW: This value doesn't seem to be cased like other query parameter values defaultValue: 'TRACKS_LAYER', validValues: SoundDesign.enumeration.keys }, diff --git a/js/quadrilateral/model/QuadrilateralShapeModel.ts b/js/quadrilateral/model/QuadrilateralShapeModel.ts index ca12ddd7..c05a5ce0 100644 --- a/js/quadrilateral/model/QuadrilateralShapeModel.ts +++ b/js/quadrilateral/model/QuadrilateralShapeModel.ts @@ -113,13 +113,13 @@ export default class QuadrilateralShapeModel { public readonly adjacentVertexMap: Map; // A map that provides the opposite vertex from a give vertex. - public readonly oppositeVertexMap: Map; + public readonly oppositeVertexMap: Map; // A map that provides the adjacent sides to the provided QuadrilateralSide. public readonly adjacentSideMap: Map; // A map that provides the opposite side from the provided QuadrilateralSide. - public readonly oppositeSideMap: Map; + public readonly oppositeSideMap: Map; // An array of all the adjacent VertexPairs that currently have equal angles. public readonly adjacentEqualVertexPairsProperty: Property; @@ -162,12 +162,10 @@ export default class QuadrilateralShapeModel { this.vertices = [ this.vertexA, this.vertexB, this.vertexC, this.vertexD ]; this.oppositeVertexMap = new Map( [ - - // REVIEW: Should the values be arrays? It looks like they are always accessed with [0]. - [ this.vertexA, [ this.vertexC ] ], - [ this.vertexB, [ this.vertexD ] ], - [ this.vertexC, [ this.vertexA ] ], - [ this.vertexD, [ this.vertexB ] ] + [ this.vertexA, this.vertexC ], + [ this.vertexB, this.vertexD ], + [ this.vertexC, this.vertexA ], + [ this.vertexD, this.vertexB ] ] ); this.adjacentVertexMap = new Map( [ @@ -184,10 +182,10 @@ export default class QuadrilateralShapeModel { this.sides = [ this.sideAB, this.sideBC, this.sideCD, this.sideDA ]; this.oppositeSideMap = new Map( [ - [ this.sideAB, [ this.sideCD ] ], - [ this.sideBC, [ this.sideDA ] ], - [ this.sideCD, [ this.sideAB ] ], - [ this.sideDA, [ this.sideBC ] ] + [ this.sideAB, this.sideCD ], + [ this.sideBC, this.sideDA ], + [ this.sideCD, this.sideAB ], + [ this.sideDA, this.sideBC ] ] ); this.adjacentSideMap = new Map( [ @@ -512,10 +510,12 @@ export default class QuadrilateralShapeModel { /** * Update a provided Property that holds a list of equal angles (either opposite or adjacent). */ - private updateEqualVertexPairs( equalVertexPairsProperty: Property, vertexMap: Map ): void { + private updateEqualVertexPairs( equalVertexPairsProperty: Property, vertexMap: Map ): void { const currentVertexPairs = equalVertexPairsProperty.value; vertexMap.forEach( ( relatedVertices, keyVertex, map ) => { - relatedVertices.forEach( relatedVertex => { + + const relatedVerticesArray = Array.isArray( relatedVertices ) ? relatedVertices : [ relatedVertices ]; + relatedVerticesArray.forEach( relatedVertex => { const vertexPair = new VertexPair( keyVertex, relatedVertex ); const firstAngle = vertexPair.vertex1.angleProperty.value!; @@ -551,11 +551,13 @@ export default class QuadrilateralShapeModel { /** * Update a provided Property holding a list of sides that are equal in length (either opposite or adjacent). */ - private updateEqualSidePairs( equalSidePairsProperty: Property, sideMap: Map ): void { + private updateEqualSidePairs( equalSidePairsProperty: Property, sideMap: Map ): void { const currentSidePairs = equalSidePairsProperty.value; sideMap.forEach( ( relatedSides, keySide ) => { - relatedSides.forEach( relatedSide => { + + const relatedSidesArray = Array.isArray( relatedSides ) ? relatedSides : [ relatedSides ]; + relatedSidesArray.forEach( relatedSide => { const sidePair = new SidePair( keySide, relatedSide ); const firstLength = sidePair.side1.lengthProperty.value; diff --git a/js/quadrilateral/view/QuadrilateralAlerter.ts b/js/quadrilateral/view/QuadrilateralAlerter.ts index 7fcc09f7..f8ab9171 100644 --- a/js/quadrilateral/view/QuadrilateralAlerter.ts +++ b/js/quadrilateral/view/QuadrilateralAlerter.ts @@ -481,7 +481,7 @@ export default class QuadrilateralAlerter extends Alerter { const currentAngle = vertex.angleProperty.value!; const previousAngle = this.previousObjectResponseShapeSnapshot.getAngleFromVertexLabel( vertex.vertexLabel ); - const oppositeVertex = shapeModel.oppositeVertexMap.get( vertex )![ 0 ]; + const oppositeVertex = shapeModel.oppositeVertexMap.get( vertex )!; const oppositeVertexAngle = oppositeVertex.angleProperty.value!; const adjacentVertices = shapeModel.adjacentVertexMap.get( vertex )!; diff --git a/js/quadrilateral/view/SideDescriber.ts b/js/quadrilateral/view/SideDescriber.ts index da1735fe..766dda2e 100644 --- a/js/quadrilateral/view/SideDescriber.ts +++ b/js/quadrilateral/view/SideDescriber.ts @@ -93,7 +93,7 @@ export default class SideDescriber { */ public getSideObjectResponse(): string { let response = ''; - const oppositeSide = this.quadrilateralShapeModel.oppositeSideMap.get( this.side )![ 0 ]; + const oppositeSide = this.quadrilateralShapeModel.oppositeSideMap.get( this.side )!; const parallelSidePairs = this.quadrilateralShapeModel.parallelSidePairsProperty.value; const thisSideIsParallel = _.some( parallelSidePairs, sidePair => sidePair.side1 === this.side || sidePair.side2 === this.side ); diff --git a/js/quadrilateral/view/VertexDescriber.ts b/js/quadrilateral/view/VertexDescriber.ts index 5b146299..e638a250 100644 --- a/js/quadrilateral/view/VertexDescriber.ts +++ b/js/quadrilateral/view/VertexDescriber.ts @@ -111,7 +111,7 @@ export default class VertexDescriber { public getVertexObjectResponse(): string { let response = ''; - const oppositeVertex = this.quadrilateralShapeModel.oppositeVertexMap.get( this.vertex )![ 0 ]; + const oppositeVertex = this.quadrilateralShapeModel.oppositeVertexMap.get( this.vertex )!; const shapeName = this.quadrilateralShapeModel.shapeNameProperty.value; const oppositeComparisonString = this.getAngleComparisonDescription( oppositeVertex, shapeName );