[wp-trac] [WordPress Trac] #46346: Page counts and related calculations typically expect and document ints but are actually floats

[WordPress Trac]

#46346: Page counts and related calculations typically expect and document ints but
are actually floats
----------------------------------------+-------------------------------
 Reporter:  lev0                        |       Owner:  SergeyBiryukov
     Type:  defect (bug)                |      Status:  reviewing
 Priority:  normal                      |   Milestone:  5.8
Component:  General                     |     Version:
 Severity:  minor                       |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:  coding-standards
----------------------------------------+-------------------------------

Comment (by lev0):

 * I'm not married to the name at all, I'm sure there's a better choice,
 perhaps something generic about splitting into groups.
 * I totally get your point of about it not being guaranteed to return a
 float and I'm fully aware that's what the last patch does. I took your
 previous feedback about the (albeit uncommon) potential for the number to
 be greater than `PHP_INT_MAX` which is why I submitted code that
 deliberately avoids casting. If you feed it ints/int-like strings (as
 documented) it gives you an int unless an input is outside the range PHP
 can store. As it is being fed these input types in the core code, so
 should the return be as expected.
 * I don't understand your point about wrapping `ceil` in `round`, my
 understanding is that [https://3v4l.org/vuJJL this is a no-op] because
 they both return floats.
 * I know how `/` works, which is why I've suggested from the beginning
 that the operation should be gently coerced into producing an int by
 giving it ints and ensuring no remainder.
 * As stated previously "This patch is not intended to guard against bad
 input (and nor does the current version)". WordPress already prevents most
 division by zero where the `ceil( $x / $y )`-style calls are currently.
 The same exceptions and errors can be thrown from the current code.
 * I didn't feel this change would have any success if it introduced a
 dependency on bcmath; I don't see any use of it in core currently (correct
 me if I'm wrong). If WordPress deems introducing it to be a good idea,
 that's fine.
 * You bring up many points about casting to an integer but also that this
 may not be desirable in case of overflow, which I agree with. Hence this
 patch is, as it was, intended to produce an int ''by default'', not
 corrupt data. I think this causes the least disruption.

 I put what I thought was a more concise function in the last patch but it
 could be (a little uglier) as before to reduce that slim chance of
 overflow:
 {{{#!patch
 Index: wp-includes/functions.php
 ===================================================================
 --- wp-includes/functions.php   (revision 50096)
 +++ wp-includes/functions.php   (working copy)
 @@ -7866,3 +7866,22 @@
  function wp_fuzzy_number_match( $expected, $actual, $precision = 1 ) {
         return abs( (float) $expected - (float) $actual ) <= $precision;
  }
 +
 +/**
 + * Calculate the number of pages a set of results can be split into.
 + *
 + * This is similar to using `ceil()` on division but returns an integer.
 + *
 + * @since 5.8.0
 + *
 + * @param int $total_items The count of items to paginate.
 + * @param int $per_page    The maximum number of items per page.
 + * @return int The total number of pages.
 + */
 +function wp_total_pages( $total_items, $per_page ) {
 +       $remainder = $total_items % $per_page;
 +       if ( $remainder ) {
 +               return 1 + ( ( $total_items - $remainder ) / $per_page );
 +       }
 +       return $total_items / $per_page;
 +}
 }}}

 I deliberately didn't submit this via GitHub as I figured I'd get feedback
 here about whatever else needed to be included, test-wise, etc. to make it
 worthwhile. I can submit it there instead if that is preferred.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46346#comment:19>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform