Skip to content

Commit

Permalink
Addressing review comments #109
Browse files Browse the repository at this point in the history
  • Loading branch information
AgustinVallejo committed Feb 28, 2025
1 parent ace2125 commit 6052c1e
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 28 deletions.
7 changes: 1 addition & 6 deletions js/photons/view/PhotonPolarizationAngleControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,7 @@ export default class PhotonPolarizationAngleControl extends Panel {
// Create a slider to control the custom angle of polarization. It is only visible when the custom preset value
// is selected.
const customAngleSlider = new HSlider( photonSource.customPolarizationAngleProperty, new Range( 0, 90 ), {

// REVIEW: Use DerivedProperty.valueEqualsConstant
visibleProperty: new DerivedProperty(
[ photonSource.presetPolarizationDirectionProperty ],
value => value === 'custom'
),
visibleProperty: DerivedProperty.valueEqualsConstant( photonSource.presetPolarizationDirectionProperty, 'custom' ),
trackSize: new Dimension2( 140, 1.5 ),
thumbSize: new Dimension2( 13, 26 ),
trackStroke: null,
Expand Down
4 changes: 2 additions & 2 deletions js/spin/model/BlockingMode.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright 2024-2025, University of Colorado Boulder

/**
* REVIEW: What does it mean for a Stern Gerlach apparatus to be blocked?
* Controls whether Stern Gerlach apparatus is blocked and which exit would be blocked
* Controls whether Stern Gerlach (SG) apparatus is blocked and which exit would be blocked.
* When a SG is blocked, one of its exits will display a wall that will prevent particles from exiting through it.
*
* @author Agustín Vallejo
*/
Expand Down
11 changes: 2 additions & 9 deletions js/spin/model/ParticleSourceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import Range from '../../../../dot/js/Range.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import Vector2Property from '../../../../dot/js/Vector2Property.js';
import Tandem from '../../../../tandem/js/Tandem.js';
import EnumerationIO from '../../../../tandem/js/types/EnumerationIO.js';
import quantumMeasurement from '../../quantumMeasurement.js';
import { SourceMode } from './SourceMode.js';
import { SpinDirection } from './SpinDirection.js';
Expand Down Expand Up @@ -55,11 +54,7 @@ export default class ParticleSourceModel {
phetioFeatured: true
} );

// REVIEW: Consider using DerivedProperty.valueEqualsConstant
this.isContinuousModeProperty = new DerivedProperty(
[ this.sourceModeProperty ],
sourceMode => sourceMode === SourceMode.CONTINUOUS
);
this.isContinuousModeProperty = DerivedProperty.valueEqualsConstant( this.sourceModeProperty, SourceMode.CONTINUOUS );

this.particleAmountProperty = new NumberProperty( 0.1, {
tandem: tandem.createTandem( 'particleAmountProperty' ),
Expand All @@ -77,10 +72,8 @@ export default class ParticleSourceModel {

const initialSpinState = SpinDirection.Z_PLUS;

// REVIEW: Recommend using EnumerationProperty instead of Property<EnumerationValue>
this.spinStateProperty = new Property<SpinDirection>( initialSpinState, {
this.spinStateProperty = new EnumerationProperty( initialSpinState, {
tandem: tandem.createTandem( 'spinStateProperty' ),
phetioValueType: EnumerationIO( SpinDirection ),
validValues: SpinDirection.enumeration.values,
phetioFeatured: true
} );
Expand Down
4 changes: 2 additions & 2 deletions js/spin/model/ParticleWithSpin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ export class ParticleWithSpin {
public velocity: Vector2;

// spin values of the particle in the XZ plane along its lifetime
// First one is initial spin after being emitted, second is exiting SG0, third is exiting SG1/SG2.
public spinVectors = [ new Vector2( 0, 0 ), new Vector2( 0, 0 ), new Vector2( 0, 0 ) ];

// REVIEW: Same as what?
// same but simplified to spinUp booleans
// same as the spinVectors but simplified to spinUp booleans
public isSpinUp = [ false, false, false ];

// whether the particle spin was already counted for the histograms or detectors
Expand Down
6 changes: 1 addition & 5 deletions js/spin/model/SpinModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ export default class SpinModel implements TModel {
phetioFeatured: true
} );

// REVIEW: Consider using DerivedProperty.valueEqualsConstant
this.isCustomExperimentProperty = new DerivedProperty(
[ this.currentExperimentProperty ],
( experiment: SpinExperiment ) => experiment === SpinExperiment.CUSTOM
);
this.isCustomExperimentProperty = DerivedProperty.valueEqualsConstant( this.currentExperimentProperty, SpinExperiment.CUSTOM );

this.particleSourceModel = new ParticleSourceModel(
new Vector2( -0.5, 0 ),
Expand Down
6 changes: 3 additions & 3 deletions js/spin/view/HistogramWithExpectedValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export default class HistogramWithExpectedValue extends QuantumMeasurementHistog
visibleProperty: expectedValueVisibleProperty
}, QuantumMeasurementConstants.expectedPercentagePathOptions );

// REVIEW: do you need to create the same shape twice or can you create it once and pass to a new Path?
const leftExpectedValueLine = new Path( new Shape().moveTo( 0, 0 ).lineTo( HISTOGRAM_SIZE.width / 3, 0 ), expectedValueOptions );
const rightExpectedValueLine = new Path( new Shape().moveTo( 0, 0 ).lineTo( HISTOGRAM_SIZE.width / 3, 0 ), expectedValueOptions );
const expectedValueShape = new Shape().moveTo( 0, 0 ).lineTo( HISTOGRAM_SIZE.width / 3, 0 );
const leftExpectedValueLine = new Path( expectedValueShape, expectedValueOptions );
const rightExpectedValueLine = new Path( expectedValueShape, expectedValueOptions );

this.addChild( leftExpectedValueLine );
this.addChild( rightExpectedValueLine );
Expand Down
2 changes: 1 addition & 1 deletion js/spin/view/SpinMeasurementArea.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default class SpinMeasurementArea extends VBox {
tandem: tandem.createTandem( 'experimentComboBox' )
} );

// REVIEW: Add documentation for this modelViewTransform. How/why is it used?
// Model View Transform to translate from model coordinates to view coordinates
const modelViewTransform = ModelViewTransform2.createSinglePointScaleInvertedYMapping(
Vector2.ZERO,
new Vector2( 0, 0 ),
Expand Down

0 comments on commit 6052c1e

Please sign in to comment.