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

Clean up usages of createObservableArray.getArray #279

Closed
samreid opened this issue Jan 18, 2020 · 4 comments
Closed

Clean up usages of createObservableArray.getArray #279

samreid opened this issue Jan 18, 2020 · 4 comments

Comments

@samreid
Copy link
Member

samreid commented Jan 18, 2020

As described in #266 (comment)

I improved documentation in the commits. I noticed there are 86 occurrences of .getArray() across the codebase. Many of these could be converted to use ObservableArray.filter and ObservableArray.map which were improved in #265 or other ObservableArray methods. Others may have simple solutions, and the ones that remain should be documented accordingly as prescribed in #266 (comment). For example, density and buoyancy has:

return _.find( this.masses.getArray(), mass => mass.isBoat() ) || null;

But this could likely be changed to something like:

return this.masses.find( mass => mass.isBoat() ) || null;

I'm not sure who should work on this or when it should be done. Should this be a chip away issue, so each developer can edit and test their own sims? @ariel-phet what do you recommend?

@samreid
Copy link
Member Author

samreid commented Jan 27, 2020

In the preceding commit, I implemented one of the conversions. I first ran the code to make sure worked correctly, and that I could understand when it was being called in the sim. Then I converted usage, and added a console.log() to double check the new code was being run in the sim. I removed the console.log before committing. When committing, I referenced this issue URL so the change is easy to track.

@ariel-phet
Copy link

@SaurabhTotey -- @samreid committed an example above for you to follow. Probably have a brief conversation with him if you get a chance to do this work.

SaurabhTotey pushed a commit to phetsims/charges-and-fields that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/bending-light that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/fractions-common that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/circuit-construction-kit-common that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/vector-addition that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/shred that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/number-line-common that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/density-buoyancy-common that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/curve-fitting that referenced this issue May 18, 2020
SaurabhTotey pushed a commit to phetsims/energy-skate-park that referenced this issue May 19, 2020
@samreid
Copy link
Member Author

samreid commented Oct 12, 2020

ObservableArray has been deleted. Let's repurpose this issue for removing usages of createObservableArray.getArray(). Be careful not to remove any calls to PhetioGroup.getArray().

@samreid samreid changed the title Clean up usages of ObservableArray.getArray Clean up usages of createObservableArray.getArray Oct 12, 2020
samreid added a commit to phetsims/balancing-act that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/equality-explorer that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/energy-skate-park that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/build-a-molecule that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/fractions-common that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/least-squares-regression that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/circuit-construction-kit-common that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/energy-forms-and-changes that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/expression-exchange that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/least-squares-regression that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/molecules-and-light that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/number-line-common that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/number-line-operations that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/number-play that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/rutherford-scattering that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/unit-rates that referenced this issue Oct 12, 2020
samreid added a commit to phetsims/vector-addition that referenced this issue Oct 12, 2020
@samreid
Copy link
Member Author

samreid commented Oct 12, 2020

I removed usages detected through aqua, as well as the getArray itself. CT may find other cases. Closing until then.

@samreid samreid closed this as completed Oct 12, 2020
samreid added a commit to phetsims/energy-forms-and-changes that referenced this issue Oct 18, 2020
jessegreenberg pushed a commit to phetsims/greenhouse-effect that referenced this issue Apr 21, 2021
@SaurabhTotey SaurabhTotey removed their assignment May 12, 2021
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

3 participants