Bug #125430 for Math-BigInt: Math-BIgFloat fails with subclassing

Tue May 29 22:51:07 2018 ylavoie [...] yveslavoie.com - Ticket created

Subject:

Math-BIgFloat fails with subclassing

Subclassing Math::BigFloat prevents accuracy, precision, div_scale... if they haven't been overloaded. The current implementation tries to access the internal values behind those getter by using the class of the calling object, assuming that it is either `Math::BigFloat` or that the parent have overloaded them and will manage to call `Math::BigFloat->accuracy()` itself. By default, non-overloaded function should default to the `Math::BigFloat` to ensure they will work and not return undefs.

Wed May 30 01:57:49 2018 peter.john.acklam [...] gmail.com - Correspondence added

This is related to CPAN RT #78097: https://rt.cpan.org/Ticket/Display.html?id=78097 I am aware of this problem. It is a result of the broken OO design in Math::BigInt, Math::BigFloat, Math::BigRat etc. It has been a problem since long before I took over as maintainer. I made a test implementation that fixes this problem, but it required a lot of changes across multiple modules. Fixing this would be nice, but that might break other modules that relies on the current behaviour, so I have been reluctant to fixing this. I haven't decided whether the best thing is to fix Math::BigInt etc. or to create a new set of distributions, for instance Math::BigInt::OO, Math::BigFloat::OO etc. Suggestions or recommendations are welcome!

Wed May 30 01:57:50 2018 The RT System itself - Status changed from 'new' to 'open'

Wed May 30 16:33:39 2018 EHUELS [...] cpan.org - Correspondence added

Hi, I'm working with Yves on the LedgerSMB project which depends on Math::BigFloat being subclassable. On Wed May 30 01:57:49 2018, pjacklam wrote: Show quoted text

> This is related to CPAN RT #78097: > https://rt.cpan.org/Ticket/Display.html?id=78097 > > I am aware of this problem. It is a result of the broken OO design in > Math::BigInt, Math::BigFloat, Math::BigRat etc. It has been a problem > since long before I took over as maintainer.

[ snip ] Show quoted text

> Suggestions or recommendations are welcome!

One suggestion I have is that you include the fact that Math::BigFloat can't be reliably subclassed as an explicit mention in https://metacpan.org/pod/Math::BigFloat#CAVEATS ; if you mean to say that in the "don't be clever" section, then that's not what I'm understanding from that paragraph. (Actually, now that I'm reading that paragraph again, I'm not sure what it's telling me...) Regards, Erik.

Wed May 30 16:47:21 2018 EHUELS [...] cpan.org - Correspondence added

Hi, [ snip ] Show quoted text

> Fixing this would be nice, but that > might break other modules that relies on the current behaviour, so I > have been reluctant to fixing this.

Can you clarify what you mean by "relies on the current behaviour"? I mean, the current behaviour is that subclasses don't work reliably, or at least that's how I read your response (in combination with our analysis of the problem so far). Obviously, nobody can truely depend on the current behaviour as their subclasses won't work. What is the change in behaviour that you implemented that other modules may depend on? Show quoted text

> I haven't decided whether the best thing is to fix Math::BigInt etc. > or to create a new set of distributions, for instance > Math::BigInt::OO, Math::BigFloat::OO etc. > > Suggestions or recommendations are welcome!

In order to determine what "other modules" there are that might rely on the current behaviour, I've queried the reverse-dependency-graph for Math::BigFloat on MetaCPAN (https://metacpan.org/requires/module/Math::BigFloat?p=1&size=100&sort=%5B%5B2%2C1%5D%5D). It seems there are 48 public modules which depend on Math::BigFloat. This seems like a mnageable set of modules we can test against the change you have developed to see what happens when you change Math::BigFloat. I'd be willing to help run the test frameworks of these modules and see what happens before and after the change you have for now, if you think that'd be an acceptable estimate of the problems that are likely to ensue from your changes. (BTW, maybe I can help with more feedback if I can see the changes you're thinking of applying. Is there anywhere I can see the new version of the code?) Regards, Erik.

Sun Jun 03 07:25:20 2018 EHUELS [...] cpan.org - Correspondence added

Hi, Studying your test-suite, I'm seeing that there's actually a test for subclassing Math::BigFloat. There's a crucial difference between that test and the documentation though: the subclass declared in the test suite declares some of the globals also declared in the original module (but the requirement to do so, isn't documented): from t/Math/BigFloat/Subclass.pm: # Globals $accuracy = $precision = undef; $round_mode = 'even'; $div_scale = 40; Although on one hand, there's an argument to be made for adding this to the documentation, every subclass should define them to work correctly. If every subclass should start out with these values defined as the defaults, there's another way to address this issue (which doesn't have the drawback of explicitly adding these variables to every subclass): If the accessors "round_mode" and "div_scale" were changed to return 'even' and 40 respectively, in case the value of the class-variables ${"<subclass>::round_mode"} and ${"<subclass>::div_scale"} respectively isn't defined --> then a modified value can still be stored in the module, but no special declaration of these values will be required in the subclassing modules. How's that for a solution? Regards, Erik.