Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Util vs Utils #966

Closed
13 tasks done
samreid opened this issue Oct 31, 2018 · 12 comments
Closed
13 tasks done

Util vs Utils #966

samreid opened this issue Oct 31, 2018 · 12 comments

Comments

@samreid
Copy link
Member

samreid commented Oct 31, 2018

From https://github.com/phetsims/phet-io/issues/1366

@pixelzoom said:

PhetioIDUtils vs PhetioIDUtil. PhET is inconsistent about whether the suffix is singular or plural. This always bites me with DOT/Util vs PHETCOMMON/StringUtils. Perhaps now is the time to standardize, since this is public facing?

@chrisklus replied:

My preference is that if the file has more than one "utility" method, then the plural suffix should be used, so *Utils.js for the above examples.

We should make a choice on this and be consistent, in the near future for PhET-iO API.

Here is a list of the current names we use for this.

Util, singular:

~/github$ find . -not -path "*node_modules*" -name "*Util.js"

  • ./dot/js/Util.js
  • ./unit-rates/js/common/URUtil.js
  • ./scenery/js/util/Util.js
  • ./scenery/js/accessibility/AccessibilityUtil.js
  • ./scenery/js/accessibility/KeyboardUtil.js
  • ./gene-expression-essentials/js/common/util/GradientUtil.js
  • ./sugar-and-salt-solutions/js/water/dev/ProjectorUtil.js
  • ./sugar-and-salt-solutions/js/utils/RandomUtil.js
  • ./interaction-dashboard/js/common/model/DataUtil.js
  • ./tambo/js/SoundUtil.js
  • ./circuit-construction-kit-common/js/CCKCUtil.js
  • ./mobius/js/Util.js
  • ./make-a-ten/js/make-a-ten/common/MakeATenUtil.js

Utils, plural:

~/github$ find . -not -path "*node_modules*" -name "*Utils.js"
./phet-io-wrapper-sonification/lib/WrapperUtils.js
./phet-io-wrapper-sonification/lib/PhetioIDUtils.js
./phet-io-wrapper-sonification/build/phet-io-wrapper-sonification/lib/WrapperUtils.js
./phet-io-wrapper-sonification/build/phet-io-wrapper-sonification/lib/PhetioIDUtils.js
./phet-io-wrapper-sonification/build/tandem/js/PhetioIDUtils.js
./phet-io-wrapper-sonification/build/phet-io-wrappers/common/js/WrapperUtils.js
./neuron/js/neuron/common/MathUtils.js
./capacitor-lab-basics/js/common/model/util/UnitsUtils.js
./installer-builder/js/DownloadUtils.js
./installer-builder/js/XMLUtils.js
./installer-builder/js/BuilderUtils.js
./installer-builder/js/DeployUtils.js
./installer-builder/js/GeneralUtils.js
./installer-builder/js/RipperUtils.js
./models-of-the-hydrogen-atom/js/common/RandomUtils.js
./scenery/js/listeners/ListenerTestUtils.js
./gene-expression-essentials/js/common/model/ShapeUtils.js
./gene-expression-essentials/js/common/model/BioShapeUtils.js
./charges-and-fields/build/wrappers/common/js/WrapperUtils.js
./charges-and-fields/build/lib/WrapperUtils.js
./beers-law-lab/build/wrappers/common/js/WrapperUtils.js
./reactants-products-and-leftovers/js/dev/DevStringUtils.js
./nitroglycerin/js/ChemUtils.js
./rosetta/js/TranslationUtils.js
./wave-interference/js/common/WaveInterferenceUtils.js
./tandem/js/PhetioIDUtils.js
./energy-skate-park-basics/build/wrappers/common/js/WrapperUtils.js
./concentration/build/wrappers/common/js/WrapperUtils.js
./concentration/build/lib/WrapperUtils.js
./color-vision/build/wrappers/common/js/WrapperUtils.js
./color-vision/build/lib/WrapperUtils.js
./circuit-construction-kit-black-box-study/build/wrappers/common/js/WrapperUtils.js
./chipper/js/common/ChipperStringUtils.js
./shred/js/Utils.js
./function-builder/js/patterns/model/FBCanvasUtils.js
./phetcommon/js/util/StringUtils.js
./john-travoltage/js/john-travoltage/view/DebugUtils.js
./john-travoltage/build/wrappers/sonification/build/phet-io-wrapper-sonification/lib/WrapperUtils.js
./john-travoltage/build/wrappers/sonification/build/phet-io-wrapper-sonification/lib/PhetioIDUtils.js
./john-travoltage/build/wrappers/sonification/build/tandem/js/PhetioIDUtils.js
./john-travoltage/build/wrappers/sonification/build/phet-io-wrappers/common/js/WrapperUtils.js
./john-travoltage/build/wrappers/common/js/WrapperUtils.js

@pixelzoom what do you recommend?

@pixelzoom
Copy link
Contributor

@chrisklus said:

My preference is that if the file has more than one "utility" method, then the plural suffix should be used, so *Utils.js for the above examples.

I don't think the name should be based on how many methods are in the file. We don't want to have to change (or remember to change) a name when the number of methods is increased from 1 to > 1.

@samreid asked:

@pixelzoom what do you recommend?

Plurals seems more correct to me. But I don't feel to strongly about this issue, if the consensus ends up being "do nothing".

@jonathanolson
Copy link
Contributor

My feelings mirror @pixelzoom's, with the note that now I prefer files named with the "theme" of what they provide. If things provide many different themes (DOT/Util), I'd prefer (long-term) to break them out into multiple files.

For example in DOT/Util, I'd want to at least separate e.g. RootSolver, Intersection or Geometry, etc. I'm not exactly sure what is ideal, but including a bunch of functions in every sim (that might be used in 1-2 sims) seems weird. The extreme (documented in phetsims/dot#4) would move things to single files, e.g. DOT/roundSymmetric (which is a style I would prefer).

@samreid
Copy link
Member Author

samreid commented Nov 2, 2018

Based on prior remarks, this seems like we have the correct name in the public PhET-iO API. Therefore, this issue would not block publication.

@samreid samreid removed their assignment Nov 8, 2018
@Denz1994
Copy link

12/13/18 dev meeting:

The majority agrees with keeping a "Utils" naming convention. @samreid has volunteered to make refactors.

pixelzoom added a commit to phetsims/unit-rates that referenced this issue Dec 13, 2018
@pixelzoom
Copy link
Contributor

@samreid FYI... I converted the "singular" list in #966 (comment) into a check list. And I took care of my sims.

@samreid
Copy link
Member Author

samreid commented Jan 22, 2019

I made these changes in my working copy and ran a full aqua an phet brand. No problems noted. I sent the following message to slack:

Very large commit incoming for #966 renaming Util => Utils. 618 files modified.

@pixelzoom
Copy link
Contributor

So everything in the checklist in #966 (comment) can be checked?

@samreid
Copy link
Member Author

samreid commented Jan 22, 2019

I think my commits + pushes will cover the entire checklist.

@samreid
Copy link
Member Author

samreid commented Jan 22, 2019

After noticing that my working copy had text like "Utilsities" in the iOS repo, I realized I'll need to try this again.

@samreid
Copy link
Member Author

samreid commented Jan 30, 2019

I message on Slack:

Sam Reid [9:31 AM]
Does everyone have the SHAs they want? #966 came up next on my TODO list.

But on second thought it will be safer to wait until after July 31. Going on hold for a while.

samreid added a commit to phetsims/circuit-construction-kit-dc-virtual-lab that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/reactants-products-and-leftovers that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/balloons-and-static-electricity that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/balloons-and-static-electricity that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/joist that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/vegas that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/tambo that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/tambo that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/shred that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/shred that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/kite that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/axon that referenced this issue Dec 30, 2019
samreid added a commit to phetsims/sun that referenced this issue Dec 30, 2019
@samreid
Copy link
Member Author

samreid commented Dec 30, 2019

Renames completed and I added a bad-text rule to prevent new modules from being named ending in Util (singular). I'll leave this open until CT has a good column or two.

UPDATE: I should note local aqua and lint are passing.

@samreid
Copy link
Member Author

samreid commented Dec 31, 2019

CT seems OK and the new lint rule seems OK, closing.

@samreid samreid closed this as completed Dec 31, 2019
jbphet pushed a commit to phetsims/number-line-common that referenced this issue Apr 1, 2020
jessegreenberg pushed a commit to phetsims/greenhouse-effect that referenced this issue Apr 21, 2021
jessegreenberg pushed a commit to phetsims/greenhouse-effect that referenced this issue Apr 21, 2021
SaurabhTotey pushed a commit to phetsims/number-line-common that referenced this issue May 12, 2021
chrisklus pushed a commit to phetsims/counting-common that referenced this issue Jun 4, 2021
chrisklus pushed a commit to phetsims/counting-common that referenced this issue Jun 4, 2021
samreid added a commit to phetsims/phet-info that referenced this issue Apr 23, 2022
zepumph pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
zepumph pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants