jrincayc / ucblogo-code

Berkeley Logo interpreter
https://people.eecs.berkeley.edu/~bh/logo.html
GNU General Public License v3.0
182 stars 34 forks source link

ISSUE-150: Validate RANDOM range input has the smaller number first #151

Closed dmalec closed 1 year ago

dmalec commented 1 year ago

Resolves #150

Summary

The RANDOM procedure does not check if the first parameter is less than the second parameter on the two input form. This change adds an explicit check and errors if the range is out of order based on the description of the procedure in the manual. It also permits RANDOM to accept a range including negative numbers.

Testing

Without this change, passing in a higher number first leads to unexpected results:

? PRINT (RANDOM 5 3)
825322468
?

After making this change, passing in a higher number will produce an error:

? PRINT (RANDOM 5 6)
6

? PRINT (RANDOM 6 5)
RANDOM doesn't like [6 5] as input

Which is equivalent to passing a negative number to the single input form:

? PRINT RANDOM 10
9

? PRINT RANDOM -10
RANDOM doesn't like -10 as input

This change also allows negative numbers in the range:

? PRINT (RANDOM -10 -5)
-7

? PRINT (RANDOM -10 10)
5

While also enforcing the ordering constraint on the range even when negative:

? PRINT (RANDOM -5 -10)
RANDOM doesn't like [-5 -10] as input

Test Environments

jrincayc commented 1 year ago

@dmalec Would you like this merged, or did you want to work on it more to fix the negative numbers problem as well?

dmalec commented 1 year ago

@dmalec Would you like this merged, or did you want to work on it more to fix the negative numbers problem as well?

@jrincayc - I'd like to work on it a bit more to fix the negative number problem; I'll mark it as in progress.

dmalec commented 1 year ago

@jrincayc - this PR now contains a proposed change for allowing negative numbers in ranges and is ready for re-review.