[ruby 2.4] Fixes for Integer#digits in PR #4375#4490
[ruby 2.4] Fixes for Integer#digits in PR #4375#4490kares merged 4 commits intojruby:ruby-2.4from whwilder:ruby-2.4
Conversation
It passes all the following tests of TestInteger of MRI: * test_digits * test_digits_for_negative_numbers * test_digits_for_invalid_base_numbers * test_digits_for_non_integral_base_numbers * test_digits_for_non_numeric_base_argument
headius
left a comment
headius
left a comment
There was a problem hiding this comment.
I know some of these style issues were in the original, but you're active so you're the lucky one :-)
| @Override | ||
| public RubyArray digits(ThreadContext context, IRubyObject base) { | ||
|
|
||
| long self = ((RubyFixnum)this).getLongValue(); |
There was a problem hiding this comment.
This is not necessary...this is already known to be RubyFixnum.
| */ | ||
| @Override | ||
| public RubyArray digits(ThreadContext context, IRubyObject base) { | ||
| BigInteger self = (this).getValue(); |
There was a problem hiding this comment.
You could just access the field here, or at least call it without the parens. I assume this is just a mistake converting the RubyFixnum-only logic.
| public RubyArray digits(ThreadContext context, IRubyObject base) { | ||
| BigInteger self = (this).getValue(); | ||
| if (self.compareTo(new BigInteger("0")) == -1) { | ||
| throw context.getRuntime().newMathDomainError("out of domain"); |
There was a problem hiding this comment.
Throw context.runtime into a local variable at the top and reuse it, in the style of other methods.
|
Thanks for the help! Squashed a few other style issues too. |
|
@headius Is there anything preventing this PR from being merged, or is it just a low priority for the moment? |
|
@whwilder great work - wanted to get |
4 participants
I've split the implementation for Integer#digits amongst Fixnum and Bignum, which should fix the issues in #4375
ref #4293