Random function with Fixes #1062
Conversation
patch that @chriseppstein wrote. Also, wrote a large expansion to the test suite.
| # Return a decimal between 0 and 1, inclusive of 0, but not 1. | ||
| # @return [Sass::Script::Number] A decimal value. | ||
| # @overload random($limit) | ||
| # Return an integer between 1 and $limit inclusive. |
There was a problem hiding this comment.
I think this is exclusive of $limit as well.
Also, $limit should be code-quoted.
There was a problem hiding this comment.
Damnit. I misread it. Here is the documentation in Ruby. Good catch, Nathan. Sorry about that. I feel like some other documentation had confused me. Changes coming. http://www.ruby-doc.org/core-2.1.0/Random.html#method-c-rand
There was a problem hiding this comment.
Oh wait. Sorry. So, the way that @chriseppstein implemented this, its actually "Random.rand($limit) + 1" So, while the ruby implementation is not inclusive, by adding one, we are making ours inclusive of the limit.
|
So, code-quoted the Also, the Random function is inclusive of $limit in this implementation, since its defined in Ruby as "Random.rand(limit) + 1". |
| if limit | ||
| assert_integer limit, "limit" | ||
| if limit.value < 1 | ||
| raise ArgumentError.new("Expected the limit to be 1 or greater") |
There was a problem hiding this comment.
For consistency, this message should be "$limit #{limit} must be greater than or equal to 1".
|
Finished reviewing. I'd like Chris's okay on this as well before it goes in. |
I read through @nex3's code comments on the random function pull request and implemented the requested changes. I also added some extra tests that weren't requested.
See original pull request here: #968