phetsims / blackbody-spectrum

"Blackbody Spectrum" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 3 forks source link

Investigate re-entrant Properties #32

Closed samreid closed 6 years ago

samreid commented 6 years ago

In https://github.com/phetsims/axon/issues/179 we identified Property instances that are re-entrant. In this context, re-entrant means a change in value of the Property causes (via listeners) another change in the value of the same Property instance.

Re-entry can occur for at least 3 different reasons, which are document here: https://github.com/phetsims/axon/issues/179#issuecomment-414717687

value=0.9999999999999998, oldValue=1 // epsilon problem
value=-0.4277580409572783, oldValue=-0.5 // large delta problem
value=[Object], oldValue=[Object] // object changed to object problem.  May be same object?

This issue is to search through the Properties with reentrant: true and:

(a) confirm that the Property really requires reentrant: true (b) identify the reason for the reentry (may be one of the 3 classes above) (c-i) see if the code can be rewritten so it no longer requires a reentrant Property, or document why the code uses a re-entrant Property or (c-ii) document why the code uses a re-entrant Property

It is unclear what the priority should be for this issue. It should probably be investigated before first RC, if not sooner.

@ariel-phet should this simulation have a responsible dev listed in https://github.com/phetsims/phet-info/blob/master/sim-info/responsible_dev.md ? It is currently blank.

ariel-phet commented 6 years ago

@arnabp this would be good for you to work on.

Feel free to ping other developers for help if this is not clear.

arnabp commented 6 years ago

Assigning to @jbphet review

jbphet commented 6 years ago

Functionally, the changes look good (and @arnabp and I discussed the issue prior to the changes). There are a few minor things that should be updated in order to meet PhET's coding style standards (see https://github.com/phetsims/phet-info/blob/master/checklists/code_review_checklist.md), and @arnabp and I will discuss in our regular weekly meeting.