JRuby

Implementing BigDecimal.mode()

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: JRuby 1.0.1
  • Component/s: Core Classes/Modules
  • Labels:
    None
  • Environment:
    Mac OS X (10.4.9), Java 1.5.0_07-164
  • Testcase included:
    yes
  • Patch Submitted:
    Yes
  • Number of attachments :
    4

Description

I've done an implementation of BigDecimal.mode(), along with tests. Comments/suggestions welcome, as this is my first stab at a significant patch and I'm pretty sure I can use some help on style.

There are two things about the MRI implementation that I learned while doing this port. One is that MRI does not respect "composition" of exception modes in a single call. In MRI, if you do BigDecimal.mode(BigDecimal::EXCEPTION_INFINITY | BigDecimal::EXCEPTION_NaN, true) the result is 2 (which is the value of BigDecimal::EXCEPTION_NaN, and which happens to be the last mode checked in the MRI code). This seems counterintuitive to me; my implementation for JRuby sets both in one call and returns 3. I could emulate MRI behavior here, but it's actually more work to do so!

The other issue is that MRI BigDecimal ignores exception modes other than BigDecimal::EXCEPTION_INFINITY and BigDecimal::EXCEPTION_NaN. There is simply no code to account for them. For consistency, I did not bother implementing the other exception modes, pending direction from the core team.

  1. BigDecimal_mode_drupp_v2.diff
    08/Jun/07 10:18 AM
    7 kB
    David Rupp
  2. BigDecimal_mode_drupp_v3.diff
    09/Jun/07 2:50 PM
    7 kB
    David Rupp
  3. BigDecimal_mode_drupp_v4.diff
    14/Jun/07 6:56 PM
    9 kB
    David Rupp
  4. BigDecimal_mode_drupp.diff
    07/Jun/07 5:59 PM
    7 kB
    David Rupp

Activity

Hide
Ola Bini added a comment -

Very good work. I'm sorry to say that I would love it if you could emulate the MRI behavior. It is idosyncracies like this that make the language we all love... =)

Show
Ola Bini added a comment - Very good work. I'm sorry to say that I would love it if you could emulate the MRI behavior. It is idosyncracies like this that make the language we all love... =)
Hide
David Rupp added a comment -

Okay – revised patch attached. Also, here is a breakdown of the exception modes defined in MRI and their status:

EXCEPTION_INFINITY, == 1, explicitly implemented
EXCEPTION_NaN, == 2, explicitly implemented
EXCEPTION_OVERFLOW, == 1, implicitly implemented (equivalent to EXCEPTION_INFINITY)
EXCEPTION_UNDERFLOW, == 4, not implemented (ignored)
EXCEPTION_ZERODIVIDE, == 1, implicitly implemented (equivalent to EXCEPTION_INFINITY)

I guess you could make a case for infinity, overflow, and dividing by zero being equivalent, computationally if not mathematically. Anyone know how/if this stuff gets used in real life? Is it worth submitting a patch to MRI to differentiate between these states, permit proper composition, and implement underflow?

Also, I couldn't find a way to invoke a class method with one required and one optional argument, which is how mode() is specified in MRI. getOptSingletonMethod() and getFastOptSingletonMethod() both specify that all arguments are optional. Should I use one of those and enforce the presence of the first method in the code? That would at least let me clean up my test cases, which currently have to pass an explicit nil as the second argument when I want it to be ignored.

Show
David Rupp added a comment - Okay – revised patch attached. Also, here is a breakdown of the exception modes defined in MRI and their status: EXCEPTION_INFINITY, == 1, explicitly implemented EXCEPTION_NaN, == 2, explicitly implemented EXCEPTION_OVERFLOW, == 1, implicitly implemented (equivalent to EXCEPTION_INFINITY) EXCEPTION_UNDERFLOW, == 4, not implemented (ignored) EXCEPTION_ZERODIVIDE, == 1, implicitly implemented (equivalent to EXCEPTION_INFINITY) I guess you could make a case for infinity, overflow, and dividing by zero being equivalent, computationally if not mathematically. Anyone know how/if this stuff gets used in real life? Is it worth submitting a patch to MRI to differentiate between these states, permit proper composition, and implement underflow? Also, I couldn't find a way to invoke a class method with one required and one optional argument, which is how mode() is specified in MRI. getOptSingletonMethod() and getFastOptSingletonMethod() both specify that all arguments are optional. Should I use one of those and enforce the presence of the first method in the code? That would at least let me clean up my test cases, which currently have to pass an explicit nil as the second argument when I want it to be ignored.
Hide
David Rupp added a comment -

One more try. My _v2 patch had a bug that would set and return 0 (zero) as the exception mode if the incoming mode was not one of the "officially" supported ones (basically, a Fixnum with value 1 or 2). Should be complete now, MRI warts and all.

Show
David Rupp added a comment - One more try. My _v2 patch had a bug that would set and return 0 (zero) as the exception mode if the incoming mode was not one of the "officially" supported ones (basically, a Fixnum with value 1 or 2). Should be complete now, MRI warts and all.
Hide
David Rupp added a comment -

Patch *_v4 includes the prior patch (implementing mode()), plus it makes mode() a method that takes one required argument and one optional, to make it consistent with MRI.

Show
David Rupp added a comment - Patch *_v4 includes the prior patch (implementing mode()), plus it makes mode() a method that takes one required argument and one optional, to make it consistent with MRI.
Hide
Nick Sieger added a comment -

Thanks again!

Show
Nick Sieger added a comment - Thanks again!

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:

Time Tracking

Estimated:
1d
Original Estimate - 1 day
Remaining:
1d
Remaining Estimate - 1 day
Logged:
Not Specified
Time Spent - Not Specified