Skip to content

Implement PHP_ROUND_UP and PHP_ROUND_DOWN options for round_updown.ph… #1658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

MarkBaker
Copy link
Contributor

PHP PR - Additional Rounding modes for the round() function

This PR adds new rounding modes to round() for PHP_ROUND_UP and PHP_ROUND_DOWN that can take advantage of the precision argument to round up or down to a specified number of decimal places.

Introduction

There has been a demand for this functionality for some time, with various of the user comments on the docs page for round() specifically requesting it, such as that posted by takingsides at gmail dot com

In my opinion this function lacks two flags:

  • PHP_ROUND_UP - Always round up.
  • PHP_ROUND_DOWN - Always round down.

In accounting, it's often necessary to always round up, or down to a precision of thousandths.

or other comment postings by others showing how the functionality can be implemented in userland code. Similar comments appear in the PHP docs pages for ceil() and floor(); and this is also a regular question tagged against PHP on StackOverflow.

While the mathematics is simple enough to implement in userland code, it would certainly be useful if it was actually implemented in PHP core, and the round() function is the logical place to do so, as it already implements precision and a mode flag that identifies a rounding method to use.

Proposal

The existing round() function already has precision logic, and provides a mode option allowing userland code to identify a method that will be applied to the rounding, but this only affects the behaviour of half (.5) values identifying whether they should be rounded up or down. Existing mode options are

  • PHP_ROUND_HALF_UP
  • PHP_ROUND_HALF_DOWN
  • PHP_ROUND_HALF_EVEN
  • PHP_ROUND_HALF_ODD

e.g

echo round( 1.54, 1, PHP_ROUND_HALF_UP);   //  1.5
echo round( 1.55, 1, PHP_ROUND_HALF_UP);   //  1.6
echo round( 1.56, 1, PHP_ROUND_HALF_UP);   //  1.6
echo round( 1.54, 1, PHP_ROUND_HALF_DOWN); //  1.5
echo round( 1.55, 1, PHP_ROUND_HALF_DOWN); //  1.5
echo round( 1.56, 1, PHP_ROUND_HALF_DOWN); //  1.6

This proposal is to add new mode options for round() (PHP_ROUND_UP and PHP_ROUND_DOWN) and the appropriate code logic to apply ceiling and floor for the value to be rounded to the precision specified in the function call.

echo round( 1.54, 1, PHP_ROUND_UP);   //  1.6
echo round( 1.55, 1, PHP_ROUND_UP);   //  1.6
echo round( 1.56, 1, PHP_ROUND_UP);   //  1.6
echo round( 1.54, 1, PHP_ROUND_DOWN); //  1.5
echo round( 1.55, 1, PHP_ROUND_DOWN); //  1.5
echo round( 1.56, 1, PHP_ROUND_DOWN); //  1.5

Behaviour for PHP_ROUND_DOWN and PHP_ROUND_UP will mimic ceil() and floor(), so PHP_ROUND_UP will round positive numbers toward infinity and negative numbers toward 0, while PHP_ROUND_DOWN will round positive numbers toward 0 and negative numbers toward -infinity.

Backward Incompatible Changes

This modification would not affect backward compatibility in any way. The existing mode flags and default behaviour would remain unchanged

Proposed PHP Version(s)

As there is no change to backward compatibility, this can be targetted at PHP.next or any intermediate release

Optionally Extending the Functionality

With minimal changes, it would also be possible to implement PHP_ROUND_TOWARD_ZERO and PHP_ROUND_AWAY_FROM_ZERO flags; where PHP_ROUND_TOWARD_ZERO would round numbers toward 0, whether positive or negative values, whereas PHP_ROUND_AWAY_FROM_ZERO would round positive values toward infinity and negative numbers toward -infinity.

Alternative Proposal

An alternative approach to provide the same functionality would be to add optional precision arguments to the ceil() and floor() functions, with a default value of 0 to ensure that backward compatibility was not affected.

References

http://www.php.net/manual/en/function.round.php
http://www.php.net/manual/en/function.ceil.php
http://www.php.net/manual/en/function.floor.php

…pt() allowing the equivalent of ceil()/floor() with a precision argument
@MarkBaker
Copy link
Contributor Author

I'm not sure that there's any value in the mode argument for

ceil(float, precision, PHP_ROUND_UP|PHP_ROUND_DOWN)
floor(float, precision, PHP_ROUND_UP|PHP_ROUND_DOWN)

because ceil() is an explicit PHP_ROUND_UP and floor() is an explicit PHP_ROUND_DOWN.

Though I have commented on the option/alternative of applying a precision argument to ceil() and floor(), I've not done any implementation of that at this time. The only potential value I see to a mode flag for those two functions would be if it were using PHP_ROUND_TOWARD_ZERO and PHP_ROUND_AWAY_FROM_ZERO to change the method of handling negative values (as described in the possible extension of functionality)

@weltling
Copy link
Contributor

weltling commented Jan 7, 2017

@MarkBaker i was just checking the PR and was about to merge it. I've adapted the patch to the current master, which is almost an easy job. Though, I've found an issue with 64-bit builds. The code is like this

printf('%f %f' . PHP_EOL, $f = PHP_INT_MAX/10002001.0, round($f, 2, PHP_ROUND_UP));

which outputs this

922152680934.022705 922152680934.030029

One would expect the output to be similar to a modified snippet like

printf('%f %f' . PHP_EOL, $f = .022705, round($f, 2, PHP_ROUND_UP));

which outputs as expected

0.022705 0.030000

Could you please check this case? It should be fixed prior merging, That would also make you to fetch the current master state in :)

Another comment yet - it could probably make sense to move the implementation into php_round_helper(), which does actual mode evaluation. This probably would integrate better with the existing code in _php_math_round().

Thanks for your work! Looking forward to merge the PR into master soon :)

@krakjoe
Copy link
Member

krakjoe commented Feb 22, 2017

@MarkBaker can we get a response to the comments here please ?

@krakjoe
Copy link
Member

krakjoe commented Jul 20, 2017

I'm going to assume at this point that this work is abandoned, and close the PR.

@krakjoe krakjoe closed this Jul 20, 2017
@weltling
Copy link
Contributor

Thanks @krakjoe. It is still interesting stuff, maybe later :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants