Assertions in BinInterface don't match documentation

[jessegreenberg]

In functions in BinInterface, there is a comment that says

@param {number} binIndex - index associated with the bin, the index may range from 0 to numberOfBins-1

But the assertions in the functions goes to numberOfBins (not numberOfBins-1)

assert && assert( binIndex <= numberOfBins );

Is the assertion incorrect or should the documentation be changed?

[veillette]

The jsDOc is correct, the assertions is wrong. It is also good practice to put a comment in the assertion. Assigning to @Denz1994.

[Denz1994]

Assertion was corrected to match documentation. Assigning to @jessegreenberg for review.

[jessegreenberg]

Thanks @Denz1994, does the same need to be done for functions getValuePosition and getBinLeft? Could you also add a comment in the assertion as suggested by @veillette in https://github.com/phetsims/plinko-probability/issues/37#issuecomment-230054462?

Also,

binIndex <= numberOfBins - 1

is equivalent to

binIndex < numberOfBins

This is really an issue of preference, but I find the second to be more clear. But totally up to you for if you want to change that.

[veillette]

I rewrote the assertion as suggested by @jessegreenberg . In the process I checked two additional assertions, and found that they were faulty and should be written without the <= sign but <.

As a sanity test I ran the sim for with assertion flag and fuzzMouse=100 for 10 minutes. No assertions were triggered. I think our work is complete here. Reassigning to @jessegreenberg.

[jessegreenberg]

Commit above looks good, thanks @Denz1994 and @veillette. Closing the issue.