phetsims / molarity

"Molarity" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/molarity
GNU General Public License v3.0
2 stars 6 forks source link

Store color strings on Solute.js instead of SoluteDescriber.getCurrentColor() #174

Closed zepumph closed 4 years ago

zepumph commented 5 years ago

From https://github.com/phetsims/molarity/issues/171

It feels much more robust to store color strings in the model, rather than have a switch case in a describer. I also feel the same way about the lowercase versions of the solute names.

This feels better to me for a few reasons:

zepumph commented 4 years ago

I implemented this above. During it I found two things that I would still like to clean up before review:

zepumph commented 4 years ago

Alright, cleanup is done now. @twant please review. Especially about the placement of these strings in the model, and if you think the new StringCasingPair data structure makes sense (and has a good name).

Thanks!

twant commented 4 years ago

@zepumph I like this a lot, particularly adding the lowercase names and colors to the model. StringCasingPair is nice, and I want to look at ways this could be used in other places where we have capitalized and lowercase string versions. I changed a few string names in this in a commit related to #172 to make sure we're consistent in identifying strings as lowercase or capitalized in the variable names (just as a flag that two versions exist, and for consistency). Thanks for doing this! Closing.