How bad are php loops

Question

I have a small function that converts a number say 1000 to 1k.

<?php

/**
 * /Converts a number into a short version, eg: 1000 -1k.
 *
 * @param int $number    The number
 * @param int $precision The precision
 *
 * @return string The formated number
 */
function short_number_format($number, $precision = 1)
{
    $lookup = [
        1 => '',
        1000 => 'K',
        1000000 => 'M',
        1000000000 => 'B',
        1000000000000 => 'T',
    ];
    $result = '';
    foreach ($lookup as $boundary => $postfix) {
        if ($number < 900 * $boundary) {
            $result = number_format($number / $boundary, $precision).$postfix;
            break;
        }
    }
    //if we didnt get the result which is most likely to happen
    // for larger numbers (rarely) ,  return the plain number

    return $result ?: number_format($number, $precision);
}

Some senior dev told me the code terribly runs slow because of the loop. How true is that?

Even if there was a problem with the code, it would likely be due to the complexity of the algorithm; not because of the loop itself. — Carcigenicate, Dec 25 '17 at 02:07
The code runs very well. Just that the dev claims that the loop slows it down — Ahimbisibwe Roland, Dec 25 '17 at 02:10
I'm surprised the dev focused on optimization and ignored the bug. — phatfingers, Dec 25 '17 at 03:46
@AhimbisibweRoland You should worry about performance if it's actually an issue. Usually there are more efficient / better ways to do things, but your standard loop (or loops) will not be perceptibly slower to the end user. You have to be doing some intense looping for the end user to take notice, for instance if you were doing a cURL call on every iteration or you had a loop that had 10000 iterations, sure, I could see there being issues...but some basic math, I dunno that it's worth it to fuss over...MHO. — Rasclatt, Dec 25 '17 at 04:40
this question suppose to be posted in codereview https://codereview.stackexchange.com/ — user1844933, Dec 25 '17 at 13:41
"the code terribly runs slow because of the loop" --- this really makes no sense. — zerkms, Dec 27 '17 at 03:08
May I ask what input values you are expecting to encounter? I am doing a lot of testing with a wide range of numbers and finding flaws in all posted methods (including mine). Will you have negative numbers? Decimals? Numbers between 0 and 1? Quintillions? — mickmackusa, Dec 27 '17 at 09:48
I'm voting to close this question as off-topic because it does not have broken code and is more suitably critiqued/optimized/reviewed on CodeReview. — mickmackusa, Dec 27 '17 at 09:51

USER249 · Answer 1 · 2017-12-25T12:59:45.110

3

Try this:

$lookup = [
    900 => '',
    900000 => 'K',
    900000000 => 'M',
    900000000000 => 'B',
    900000000000000 => 'T',
];

$result = '';
foreach ($lookup as $boundary => $postfix) {
    if ($number < $boundary) {
        $result = number_format($number / ($boundary/900), $precision) . $postfix;
        break;
    }
}

This way you perform divison ONE time instead of multiplication EVERY time.

Edit: Improved answer:

Since Division cost more processing than Multiplication, we better change it to:

$result = number_format(($number*900) /$boundary, $precision) . $postfix;

this should theoretically be faster and CONSIDERABLY FASTER if your short_number_format() function is called from another loop with large number of iterations

edited Dec 25 '17 at 12:59

answered Dec 25 '17 at 02:58

USER249

1,080
7
14

"Since Division cost more processing than Multiplication" --- this needs some evidence. – zerkms Dec 27 '17 at 03:10
It's also just common sense! when you multiply you only add, however when you divide you subtract with the probability of reminder to check for and deal with. – USER249 Dec 27 '17 at 03:37
1

I asked it so that you added it to the answer, not because I needed the explanation. – zerkms Dec 27 '17 at 05:52
1

By "you" I mean the reader in general and not you as "zerkms". Excuse me. – USER249 Dec 27 '17 at 15:40

phatfingers · Accepted Answer · 2017-12-25T19:16:04.340

Nothing about this code appears to me to raise a performance concern. A fixed-length loop is an O(1) operation and a couple of basic math calculations simply would not be the root cause of a performance problem.

I'd fix the typo in your postfix for Gigabytes and consult with your dev to see whether his/her concern is because a performance problem was actually seen at runtime. Maybe you could offer to put some timers in to measure what operations are using the most time. You could also create a unit test to verify what you already know intuitively about your own function's performance.

Addition: I created a string-based version that uses no loops and timed it against your version. While the no-loop version may run very slightly faster, you'll find that your existing version can already process hundreds of thousands of requests per second on a fairly modest CPU. A micro-optimization of your function is unlikely to have any noticeable impact.

<!DOCTYPE html>
<html>
<body>
<?php
$t1 = microtime(true);
for ($i=0; $i<100000; $i++) {
    scaleV1($i, $i%3 + 1);
}
timesince($t1);
$t2 = microtime(true);
for ($i=0; $i<100000; $i++) {
    short_number_format($i, $i%3 + 1);
}
timesince($t2);


function timesince($micro) {
    $sec=(microtime(true) - $micro);
    echo "<p>" . number_format($sec, 3) . "</p>\n";
}


function scaleV1($number, $precision=1) {
    $scale_id=" KMGTP";
    $w=strlen(intval($number));
    $scale=intval($w/3);
    if (($w%3==0) && (substr($number,0,1)!=9)) $scale--;
    $postfix=$scale==0 ? '' : $scale_id[$scale];
    return number_format(($number / pow(1000,$scale)), $precision) . 
$postfix;
}

function short_number_format($number, $precision = 1)
{
    $lookup = [
        1 => '',
        1000 => 'K',
        1000000 => 'M',
        1000000000 => 'G',
        1000000000000 => 'T',
    ];
    $result = '';
    foreach ($lookup as $boundary => $postfix) {
        if ($number < 900 * $boundary) {
            $result = number_format($number / $boundary, 
$precision).$postfix;
            break;
        }
    }
    return $result ?: number_format($number, $precision);
}
?>

</body>
</html>

Your assumption that this is calculated bytes may be incorrect. My assumption is that these postfix values merely represent thousands, millions, billions, trillions, etc. So you can think of the input numbers as poker chips or SO rep points if you like. — mickmackusa, Dec 28 '17 at 04:04

USER249 · Answer 3 · 2017-12-28T18:25:16.727

This is another way without looping nor branching:

 function short_number_format($number, $precision = 1)
{
    $lookup = [
               1 => [1,''],
               2 => [1,''],
               3 => [1,'']
               4 => [1000,'K'],
               5 => [1000,'K'],
               6 => [1000,'K'],
               7 => [1000000,'M'],
               8 => [1000000,'M'],
               9 => [1000000,'M'],
              10 => [1000000000,'B'],
              11 => [1000000000,'B'],
              12 => [1000000000,'B'],
              13 => [1000000000000,'T'],
              14 => [1000000000000,'T'],
              15 => [1000000000000,'T']
        ];

       $N = floor(log($number, 10) + 1);
       $result=$number/$lookup[$N][0]; 
return number_format ($result,$precision) . $lookup[$N][1];
}

In this method we use the number of digits as index to the lookup array and use 0 and 1 to get the divisor and the letter respectively.

another way is to use (only if $number is double or float containing dot):

$N = strpos(strval($number),".")-1; // only if $number has dot like a double

-EDIT another method using string functions instead of math assuming input number is a double with a dot (not tested):

function short_number_format($number, 
$precision = 1)
{
$lookup = [
           1 => [0,''],
           2 => [0,''],
           3 => [0,'']
           4 => [3,'K'],
           5 => [3,'K'],
           6 => [3,'K'],
           7 => [6,'M'],
           8 => [6,'M'],
           9 => [6,'M'],
          10 => [9,'B'],
          11 => [9,'B'],
          12 => [9,'B'],
          13 => [12,'T'],
          14 => [12,'T'],
          15 => [12,'T']
    ];
$sn = strval($number);
$N = strpos($sn,".") - 1;
$pos = $N - $lookup[$N][0];
$sn = str_replace('.', '', $sn) ;
$sn = substr_replace($sn, ".", $pos, 0);
return number_format (doubleval($sn),$precision) . $lookup[$N][1];


}

Added non-branching argument after reading an answer with more than 20K upvotes, it states that:

A general rule of thumb is to avoid data-dependent branching in critical loops

reference: https://stackoverflow.com/a/11227902/6281135

also from the same thread it seems that sorted data runs faster if you want to keep the IF statment. By data i mean the data passed to your function in case you are calling it from another loop.

How bad are php loops

3 Answers3