Number::Fraction

[James Laver]

Jesus, I thought my mail was long…

Apols in advance...

On 6 May 2013, at 21:13, Th. J. van Hoesel <th.j.v.hoesel at gmail.com> wrote:

> After reading it three times, I have a slight feeling that I have bitten of more then I can chew... and it was such a small bite already, couldn't find a smaller module, so little code and such a elegant way of doing what it is supposed to do - and I have some kind of affinity with as former math-teacher.

It is a fairly trivial module and I don't think wrong for your first chunk of real world perl to be honest, I just think perhaps a little guidance could help.

> And the documentation does say what parameters it takes, does not say anything about what it does when passing in additional things.

Well then, it's "undocumented behaviour", which should not be relied upon. I don't personally see any harm in being liberal in what you accept.

> I will admit, working with positional parameters is bad, and I do acknowledge the danger in just adding other parameters and mess up the thing. Particular, like stated before, Perl takes any number of parameters in function-calls, and it would not be fun to have to reorder the parameters in the function call anytime someone comes up with a new idea.

Well quite.

> But it doesn't --- or, I have not tested it, but the code does explain. It happily keeps running, though it is not conform the documentation

Sorry, how does it not conform to the documentation exactly? My understanding here was that it didn't document what it did with extra params and thus a fairly reasonable response was to simply ignore them?

> So, I should fill in an official bug ?

I remain unconvinced there is a bug. Unless you've tickled one with some code you've got in production, I don't think a bug is worth filing.

> Adding
> if ( @_ > 2 ) warn "fractions consist of a numerator and a denominator, found more than 2 parameters\n";
> would be acceptable?

It seems like a reasonable position to me. A warning never hurts anyone who isn't doing stupid things with %SIG.

>> Why don't you just create a new constructor that takes arguments differently? This would seem to be a good way around it. Seems to me you're way overcomplicating things. It's also a common perl pattern because we're blessed with the ability to have as many constructors as we want.
> 
> that could be an option.
> 
> I'd like to keep the module as it is... but accept other arguments, atop of what has already been implemented. The module accepts in one single method 3 different calls:
> 
> passing a reference, which clones it to a new object.
> 
> passing a string, which will be unraffled in a numerator and denominator.
> 
> passing in 2 integers to construct the object.
> 
> these are not 3 separate constructors.
> I would like expand the string handling and allow it to pass in a string in the form off "a b/c". That is not a great deal, just a bit off code. However, I think it then looks a bit off to need an additional routine Number::Fraction->new_from_integer_numerator_and_denominator( a, b, c ). Therefor, I'd like to be able to use the same constructor that works with 2 AND 3 parameters, I think that would look better, building on top off the overload features.

It's not my module, I've never used it before and I personally couldn't care less if you make it accept three params optionally and break code where people aren't following the docs. But don't shove the new argument on the front because it's a terrible thing to do. Also, I'd hardly see 

> 
>>> next:
>>> let the module die with unintended 3-plus-argument calls
>>> use Number::Fraction qw{:mixed}
>> 
>> Again, you're overcomplicating.
> 
> May i quote:  "Furthermore, if programs are using the module aside from how it's documented, they deserve to be broken."

You've well over-engineered this. Break things for the right reason, not because you've got a batshit crazy way of solving a problem.

> I'm only trying to phase out the undocumented possibility that the method could be called with to many parameters, that would potentially cause harm the moment you would reorder the arguments in the call.
> 
> I am aware of the danger now.
> 
> Had it been caught since the early releases, there might not had been any calling programs that would do ...new->( 2, 5, 7);

Why is this a problem? Why can't you just work around it like everyone else does?

Frankly have you considered making a subclass that handles three args? It might be a better fit.

> Doing Objective-C as well - which is not just a new kid in town either - that does not use named parameters either, it does positional parameters to their method calls. However, through some smart signature, it uses extreme long function-calls that makes it all quite explicit and does do quite a lot of parameter checking in the IDE. Guess the only way to do real named parameters is through NSDictionary or what we in Perl have, called a hash, in which case the order does not affect the result.

And here enters Moose.

> That is what have come aware off, realising people might 'abuse' the method, hence my approach to phase out bad code, by 'alerts' and motivate people to check their code, allow them time to update and then gradually make the move to the new API. I know it's tricky.

As I said, I've got no problems with breaking code when people are doing things wrong, but it doesn't mean you shouldn't in general try and avoid it. And there are so many ways to do that that are less broken.

> If this is not a right reason, what would for example be legated reasons to break code, just curious.

And what colour should we paint the bike shed this year? I hear black is back in.

Personally I bend over backwards to be backwards compatible if I know people already use my modules. In this case, it's quite clearly in violation of the docs and I'm not sure I give two shits either way, particularly since I've never used the module in question.

> Accept from the question if it is the right way... what would become confusing in this constructor is the 'natural' order of human perception:
> 
> to construct a three-quarter:
> my $threequarter = Number::Fraction->new( 3, 4 ); # numerator / denominator
> 
> to construct two three-quarter:
> my $badthreequarter = Number::Fraction->new ( 3, 4, 2); # bad natural order, numerator, denominator, integer
> 
> that should be:
> my $twothreequarter = Number::Fraction->new ( 2, 3, 4 ); # human natural order
> 
> like it is in Math::Fraction as well.

If you want that, just make another constructor. Personally I can't bring myself to see it natural in any order.

> I will definitely have a look in the Moose branch, and will assure that both modules will have comparative methods and features


> Now, it all boils down to:
> 
> Had Number::Fraction->new not been so kind to allow silly function-calls with the abundance of additional parameters, and only accepted the 2 parameters (or 1, or zero), then there could not be any mistake implementing a new version that would do the right thing passing in 3 parameters (in human order) for there would not be any application that would have already a three-parameter constructor call.

Go to CPAN right now and pick a module at random. I'd say there's an 80% chance it has the same behaviour. You consider this a bug, but you're alone in that view. I don't consider 'human order' a good enough excuse to mess with param ordering and I don't consider that this behaviour needs to be wrapped into the same constructor.

> Implementing a 3 parameter version, guess, it will for now only be a good way to do it to have 'use Number::Fraction qw/:mixed/'

Sorry, that's not a good solution. I've presented several and I'm not quite sure why none of them seems right to you.

> and please, anybody out there, shoot this essay to pieces and tell me what I do harm to the Perl community - I will really appreciate it... and you will do yourself and others a favour, raising me to the next level and preventing horrible things to seep into the CPAN
> 
> =[ START READING HERE ] =================================================================================
> 
> And lastly, just before I was ready to send this essay...
> 
> It's not even the argument if the 3rd parameter, the integer part of the mixed fraction, should be at the end (for the sake of Perl programmers) or at the beginning (for the sake of humans). Any legacy code in any application is at risk the moment one would allow a third parameter.

So don't disturb it?

> take for example the $badthreequarter...
> 
> before the enhancement...
> 
> my $badthreequarter = new->( 3, 4, 5); # which allowed additional useless parameters
> my $oldresult = 4 * $badthreequarter; # == 3
> 
> after the enhancment...
> my $badthreequarter = new-> (3, 4, 5); # which would be 5 3/4 in case of the 3rd parameter be at the end (bad idea still)
> my $newresult = 4 * $badthreequater; # == 23
> 
> so... even if the 3rd parameter would be at the end, ANY nasty application that used additional useless parameters will break, no matter if it would be at the end or the beginning.

Yes. So either break it or don't break it. Just don't break it in a really quite ridiculously overcomplicated way.

> Having said that, I think it should be the first step to eliminate that behavior and put in the alert. Although it 'should' break.
> 
> The 3-parameter version will need to import :mixed
> 
> and hopefully, after a long time, we can assume that no one uses the old version that had caused warnings, and can then drop the import

Or, y'know, just use a sensible alternative. I believe I've outlined several. Just to recap:
- Second constructor
- Subclass
- Moosify and only support the old type with positional params

James