Re: Incorrect terminology usage for rounding modes of round()

On 14 April 2024 16:55:43 BST, Saki Takamachi <saki@sakiot.com> wrote: >Hi Gina, > >> Hello internals, >> >> While reviewing the PR for the "Adding bcround, bcfloor and bcceil to BCMath" RFC >> [1] with the different rounding modes, >> I was made aware of the unfortunate wrong terminology usage of PHP_ROUND_HALF_UP and >> PHP_ROUND_HALF_DOWN. >> >> Indeed, PHP_ROUND_HALF_UP is implemented as "rounding half away from zero" and >> not "rounding half up" (which is also called rounding toward positive infinity). >> The behaviour for positive numbers is the same, however for negative numbers the rounding >> *is* different. >> In the same vein, PHP_ROUND_HALD_DOWN is implemented as "rounding half toward >> zero" and not "rounding half down" (/round half toward negative infinity). >> >> Taking -1.5 as our number: >> - Rounding half-up: -1 >> - Rounding half away from zero: -2 >> - Rounding half-down: -2 >> - Rounding half towards zero: -1 >> >> For a detailed explanation about rounding, the Wikipedia page is of great use. [2] >> And the following rounding calculator on Calculator Soup is useful to check the differences >> and behaviour of different rounding modes. [3] >> >> It should be noted that PHP is in good company about being wrong about those two rounding >> modes, Java, Python, and Ruby (and probably others) are also wrong in this regard. >> >> Considering that PHP 8.4 is also adding new rounding modes via the "Add 4 new rounding >> modes to round() function" RFC [4] attempting to solve this issue in this next version of PHP >> seems like a good idea. >> In my discussions with Saki about this issue, it seems that her and Tim have thought about >> creating a new enum for rounding modes, looking something like this: >> >> enum RoundingMode { >> case HalfAwayFromZero; >> case HalfTowardsZero; >> case HalfEven; >> case HalfOdd; >> case TowardsZero; >> case AwayFromZero; >> case NegativeInfinity; // or case Floor; >> case PositiveInfinity; // or case Ceiling; >> } >> >> and change the signature of round from: >> round(int|float $num, int $precision = 0, int $mode = PHP_ROUND_HALF_UP): float >> to >> round(int|float $num, int $precision = 0, RoundingMode $mode = >> RoundingMode::HalfAwayFromZero): float >> >> and changing the definition of the existing constants to effectively be: >> define('PHP_ROUND_HALF_UP', RoundingMode::HalfAwayFromZero); >> define('PHP_ROUND_HALF_DOWN', RoundingMode::HalfTowardsZero); >> define('PHP_ROUND_HALF_EVEN', RoundingMode::HalfEven); >> define('PHP_ROUND_HALF_ODD', RoundingMode::HalfOdd); >> >> This should not cause any BC breaks, while allowing us to potentially implement the half >> up/down rounding modes properly, and deprecate the existing rounding constants in the future to get >> rid of the confusing names. >> >> I wanted to know if anyone has any object to introducing this new enum and signature >> change. >> The only thing I could think of is if this enum should be in a new Maths (or Math or just >> Mathematics to not need to deal with regional difference in the short spelling of >> "Mathematics") namespace. >> >> Best regards, >> >> Gina P. Banyard >> >> [1] >> https://wiki.php.net/rfc/adding_bcround_bcfloor_bcceil_to_bcmath >> [2] https://en.wikipedia.org/wiki/Rounding >> [3] >> https://www.calculatorsoup.com/calculators/math/rounding-methods-calculator.php >> [4] >> https://wiki.php.net/rfc/new_rounding_modes_to_round_function > >In my discussions with Tim, I want to make it clear that all ideas derived are Tim's. :) > >Now, of course I agree with this, and agree with using namespaces. However, if didn't use >namespaces, that's fine by me. For reference, we found approximately 100 pieces of code using >the namespace Math on Github Code Search. There are only a few cases >for Maths and 0 for Mathematics. > >Regards, > >Saki Math is American English spelling, Maths/Mathematics the British English one. As we also have color (and not the "correct" colour), going with the American spelling seems better. But I would query whether we actually need it in a namespace. cheers Derick