max(new Decimal(NAN), new Decimal(-INF)) give not correct result (-INF)

[FoxKeys]

As we discussed in #https://github.com/php-decimal/ext-decimal/issues/10 NAN is not comparable, must raise and exception OR return NAN Because signum() have INT result, you decide to raise an exception for signum() (and i agree with this). But, in case of min/max return of NAN seems to be better solution.

Important! I'm not sure that min()/max() assumed to be used with Decimal at all. If so - then this issue must be fixed. If Decimal not allowed to be used with min()/max() functions - then raise and exception would be good if one (or both) of argumens for min/max is Decimal. And, if Decimal wil be forbidden argument for PHP min()/max() then new Decimal->min(...)/Decimal->max(...) would be good idea for Decimal API.

P.S. Just info to @rtheunissen. min/max is last test case in our project (at least for now), so no new issues expected from me :). And, i can send you PHPUnit test file for our Decimal wrapper. Maybe you will find something interesting ;) P.P.S. I'm pretty happy with your fast answer/fixes.

Best regards, Sergey

[rtheunissen]

The current behaviour around NAN is that any comparison returns 1. A comparison must return an integer, so PHP uses 1 if comparison is undefined. https://3v4l.org/84IC7

I don't think you should be using any php functions with this library. Many of them do not consider objects or will implicitly cast to float. I advise that you write your own function to achieve min/max.

If we did implement min/max, we should do so on a standardized math library under the decimal namespace that always returns Decimal. This would include functions like min, max, sum, avg, sin, cos, tan, etc.

[FoxKeys]

Yes. For "usual" objects this is correct. But, i suppose the library overrides (overloads) comparision perators! Is it not?

[rtheunissen]

It overrides the comparing behaviour of the Decimal object, but not comparison in PHP itself. This extension has no vision into the min or max functions. Comparing context is not known.

[rtheunissen]

I would like to see your wrapper, by the way. I'm curious to take a look. 😊

[FoxKeys]

Sorry, i'm never going deep into PHP/extension internals (just not enough enough free time...) So, don't know how operator overload works. Just know "PHP have such option".

Therefore, i must add own min/max functions to wrapper. Thank you for answer.

P.S. I'll send you wrapper and tests soon (monday, i suppose). Wrapper it self is not interesting. But tests.

[rtheunissen]

Feel free to close. :)

PS: Check out the 2.0 branch also - some very nice performance improvements there.

[FoxKeys]

Hello, Rudi. MonetaryDecimal.zip

Source  attached (not sure it is pass github mailer, but try). I hope you find something interesting for you.

For example, "execLambdaTest"  in  tests  are interesting, as for me (flexible thing, allow to test functions with any arguments count) Writing same in $this->assertSame((string)self::FLOAT_NEGATIVE, (new MonetaryDecimal(self::FLOAT_NEGATIVE))->toString()); style is much, much worst (long and bad readable). Especially, with many arguments. But, this is just IMHO.

Anyway, enjoy. And thanks for you work with Decimal.

Best regards, FoxKeys                          mailto:FoxKeys@Mail.ru

[FoxKeys]

P.S.

PS: Check out the 2.0 branch also - some very nice performance improvements there.

Unfortunately, i can't. Our ITOps installs only pre-built production packages (pecl install decimal), so onl 1.3 are installed.

[rtheunissen]

@FoxKeys you will find significant benefits from using 2.0 by looking at your code. :sparkles:

I'll try to get a 2.0.0-alpha released on PECL soon.