Number::Fraction

[Th. J. van Hoesel]

James, thank you so much for writing such a great email.

=[ PLEASE JUMP TO THE END OF THIS MAIL ]=============================================

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.

You say 'Overcomplicate' a few times, maybe yes.

Upgrading this module was merely supposed to be a good 'exercise' to start understanding the problems of real-world Perl Programming, but please understand that I might ask some questions and feedback on things so obvious for most of the guru's, but not for me. And it's only through your and other generous advice that I find myself improving my skills.

And I will admit, I do not easy take an answer, rather have a little discussion that will convince me off the pro's and on's and get to understand why, rather than just accepting and having nothing learned.


Now back to your much appreciated response.

Op 6 mei 2013, om 16:34 heeft James Laver het volgende geschreven:

> On 6 May 2013, at 13:57, "Th. J. van Hoesel" <th.j.v.hoesel at gmail.com> wrote:
>> 
>> What if people figured out that the method did accept 3 or more arguments (why and how.. because they dug into the code ?). It would make no sensense at all to call the method Number::Fraction->new( 1, 4, 6, 7, 2), but it would produce by some magic allowance that it should return a Number::Fraction object.
> 
> Okay, well first of all any constructor is going to take an arbitrary number of arguments. That's how argument passing in perl works.

Urm - yep... totally right.

> If they read the code they would see what it does, the only way they would otherwise see it taking more than two arguments is by attempting to pass them. And if they did that, they ought to have been reading the documentation more closely.

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

> Otherwise they did and they just decided to see what happens? I don't see that one myself.

Why on earth would one try, i all that was needed was a module that does "1/2" or ( 1 , 2 ), Nope, don't see that one myself either.

> 
>> So, now we have some legacy code. And now extending the functionality in a natural understandable manner, might cause old programs that did very odd method calls to break.
> 
> I think you misunderstand here. How is what you propose a natural understandable manner? Positional parameters are by convention added to the end if you're creating more of them, not the front. See what messes libraries in other languages have gotten into by making that mistake (erlang springs to mind).

Nothing springs to mind.

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.

> Furthermore, if programs are using the module aside from how it's documented, they deserve to be broken.

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

> They could have submitted a bug to make something official behaviour, but they didn't. And since we're talking about a simple fraction module here, I don't think we're going to find anyone doing something daft.

So, I should fill in an official bug ?

>> Is my next approach acceptable:
>> 
>> now:
>> a quick patch that will issue a warning the moment the method is being called with 3 or more arguments (only once per application run), letting users know that they did some odd thing, but forgive them and continue.
> 
> Nondestructive. I don't think anyone would argue with this.

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

>> next:
>> implement a 3-argument methode that will only work properly when the modules is being used as
>> use Number::Fraction qw{:mixed}
>> still issue a warning
> 
> 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.

>> 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."

> 
>> next:
>> drop the requirement to use :mixed, make it default
>> 
>> all in a time frame of one year ?
> 
> Why on earth are you overcomplicating this so?

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);

>> Someone really need to enlighten me as to why write method implementations like this, that accept arbitrary arguments, rather than locking down the API
> 
> It's a dynamic language. We have documentation on how to do things. And extra arguments are ignored. This is fairly normal. I write a lot of code that does this too and i don't consider it broken. In fact, it's more or less a perl idiom and common to a lot of dynamic languages really (though in the majority of cases they just use named parameters because the language has native support for method signatures).

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.

> Ask yourself what you're achieving by changing the API like this. I think you've got the potential to break code

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.

> and I don't think you're doing it for the right reasons.

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

> I don't like changing the order of positional parameters when you're adding new ones either because it confuses people.

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.

> Furthermore, I don't think what Dave is suggesting about the move to Moose is a daft one, but you'd do well to preserve the API as it currently stands. Perhaps a BUILDARGS method that checks if it's being called according to the old API (i.e. has two args, both of which are numeric) and fills in the keys for you if so? To get the new 'mixed' functionality, you'd just pass in the keys manually and it would do the right thing?

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

> James

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.

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/'

Greeting Theo...

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.

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.


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