search

phetsims / dot

A math library with a focus on mutable and immutable linear algebra for 2D and 3D applications.
MIT License
13 stars 6 forks source link

Code review for Random.js #63

Closed samreid closed 8 years ago

samreid commented 8 years ago

Since Random.js will be more widely used (via phet.joist.random), it should be reviewed. @samreid should prepare it for review, then it would be nice if @pixelzoom has time to take a closer look.

samreid commented 8 years ago

I've finished my cleanup, @pixelzoom are you available to review Random.js?

pixelzoom commented 8 years ago

Cleanup looks great.

There is an unrelated issue, probably introduced when shuffle was implemented. IDEA is flagging this line as a potential issue, "Constructor called without new"

133 var result = Array( array.length );
samreid commented 8 years ago

Agreed, we should use new Array, fixed above and it looks good in testing. Anything else here?

pixelzoom commented 8 years ago

Nothing else, closing.