search

phetsims / gene-expression-essentials

An educational simulation about how genes work to create proteins.
GNU General Public License v3.0
4 stars 6 forks source link

getGeneAtPosition returns the last match instead of the first match #138

Closed samreid closed 3 years ago

samreid commented 3 years ago

Discovered in https://github.com/phetsims/chipper/issues/1033, it seems the intent of the code is to return the first match, but since an arrow function is used instead of a for loop, it actually returns the last match.

@jbphet can you please review and advise?

samreid commented 3 years ago

I disabled the consistent-return lint rule for now, until @jbphet can take a look.

jbphet commented 3 years ago

I've changed this to a C-style loop and used the geneAtPosition value as part of the continuation clause. I tested by running the sim, setting breakpoints, and walking through the method when transcribing genes. I transcribed all genes on the first two screens (no transcription is done on the 3rd screen).

I think that this never caused any problems because there is ever only one gene that contains a base pair, but it's definitely an improvement to fix the code.

I don't think there is a need for a review here, so I'm going to close the issue. @samreid is welcome to take a look and re-open if it seems necessary.