0
\$\begingroup\$

I created a function that will tell me how many times a number is divisible by another number and what the remainder of it is:

 <?php
function divisible_times($number, $divider) {
    $count = 0;
    $remainder = $number % $divider;
    $num_array = range($number, 1);
    foreach($num_array as $num) {
        if($num % $divider == 0){
            $count++;
        }
    }

    $arr[$number.':'.$divider]['divisions_no'] = $count;
    $arr[$number.':'.$divider]['remainder'] = $remainder;
    return $arr;
}
print_r(divisible_times(4, 3)); // Returns Array ( [4:3] => Array ( [divisions_no] => 1 [remainder] => 1 ) )
print_r(divisible_times(14, 3)); // Returns Array ( [14:3] => Array ( [divisions_no] => 4 [remainder] => 2 ) )
print_r(divisible_times(13, 3)); // Returns Array ( [13:3] => Array ( [divisions_no] => 4 [remainder] => 1 ) )
print_r(divisible_times(12, 3)); // Returns Array ( [12:3] => Array ( [divisions_no] => 4 [remainder] => 0 ) )

Can this code be optimized somehow? I tried to use $number directly, but couldn't find out the count that way. The only thing I could think is to create am array of numbers and go through each one.

I mean this does the trick, but optimization is important, and I was wondering if there is any other way of doing it?

\$\endgroup\$
4
\$\begingroup\$

Try the following:

function divisible_times($number, $divider) {
  $arr[$number.':'.$divider]['divisions_no'] = floor($number / $divider);
  $arr[$number.':'.$divider]['remainder'] = $number % $divider;
  return $arr;
}
\$\endgroup\$
1
\$\begingroup\$

I think you already got a good comment from @jedrzej.kurylo about how to simplify the operation itself.

  • I would suggest enforcing the input values as integer (since modulus, in essence, only operates on integers), and prevent divide by zero cases.

  • I would also add that your return data structure seems odd. Why have an array key like $number.':'.$divider? What value does it add since the calling code already knows the values that were passed for the numerator and divisor. It is also unclear why you would want to return a two-dimensional array here.

  • Finally, I would use more meaningful variable names that convey more specific mathematical meaning.

Building on code from @jedrzej.kurylo and my suggestion above, you might have something like:

function divisible_times($numerator, $divisor) {
    if(empty($numerator) || !is_int($numerator)) {
        throw new InvalidArgumentException(
            'Invalid value passed for $numerator parameter. ' .
            'Integer value expected. Value provided: ' . 
            var_export($numerator, true)
        );
    }
    if(empty($divisor) || !is_int($divisor)  || $divisor === 0) {
        throw new InvalidArgumentException(
            'Invalid value passed for $divisor parameter. ' .
            'Non-zero, integer value expected. ' .
            'Value provided: ' . var_export($divisor, true)
        );
    }

    // build object for return
    // you could just as easily build associative array here
    $return = new stdClass();
    $return->quotient = floor($numerator / $divisor);
    $return->remainder = $numerator % $divisor;
    return $return;
}
\$\endgroup\$
  • \$\begingroup\$ A really nice solution, but my 'motives' for this function were to help me in building a HTML structure based on the number of posts in WordPress. That's why the names are not that important to me (the names of the resulting keys in the array, only the content matters) :) \$\endgroup\$ – dingo_d Jun 13 '16 at 17:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.