Calculate a median

Question

I got this as an implementation of "get me the median of those values". But it sort of doesn't feel right (too long, too many branch points) so I thought I'll post it here to see what you think.

<?php
private function calculateMedian($aValues) {
    $aToCareAbout = array();
    foreach ($aValues as $mValue) {
        if ($mValue >= 0) {
            $aToCareAbout[] = $mValue;
        }
    }
    $iCount = count($aToCareAbout);
    sort($aToCareAbout, SORT_NUMERIC);
    if ($iCount > 2) {
        if ($iCount % 2 == 0) {
            return ($aToCareAbout[floor($iCount / 2) - 1] + $aToCareAbout[floor($iCount / 2)]) / 2;
        } else {
            return $aToCareAbout[$iCount / 2];
        }
    } elseif (isset($aToCareAbout[0])) {
        return $aToCareAbout[0];
    } else {
        return 0;
    }
}

Answer 1

The first part of your function filter out negative values. This has nothing to do with calculating the median itself, so should be moved away from this function.

Way I would do it.

Create array_median() function in a global scope (or a static method) like this:

/**
 * Adapted from Victor T.'s answer
 */
function array_median($array) {
  // perhaps all non numeric values should filtered out of $array here?
  $iCount = count($array);
  if ($iCount == 0) {
    throw new DomainException('Median of an empty array is undefined');
  }
  // if we're down here it must mean $array
  // has at least 1 item in the array.
  $middle_index = floor($iCount / 2);
  sort($array, SORT_NUMERIC);
  $median = $array[$middle_index]; // assume an odd # of items
  // Handle the even case by averaging the middle 2 items
  if ($iCount % 2 == 0) {
    $median = ($median + $array[$middle_index - 1]) / 2;
  }
  return $median;
}

This way we have generally available all purpose function, with naming consistent with core php functions.

And your method would look like

/**
 * The name should probably be changed, to reflect more your business intent.
 */
private function calculateMedian($aValues) {
  return array_median(
    array_filter(
      $aValues, 
      function($v) {return (is_numeric($v) && $v >= 0);}
      // You can skip is_numeric() check here, if you know all values in $aValues are actually numeric 
    )
  );
}

Either within calculateMedian() or in the code that calls it, you should take care of catching the DomainException that can be thrown if the array is empty)

Answer 2

I'm wondering if you can just compact the above to this:

private function calculateMedian($aValues) {
    $aToCareAbout = array();
    foreach ($aValues as $mValue) {
        if ($mValue >= 0) {
            $aToCareAbout[] = $mValue;
        }
    }
    $iCount = count($aToCareAbout);
    if ($iCount == 0) return 0;

    // if we're down here it must mean $aToCareAbout
    // has at least 1 item in the array.
    $middle_index = floor($iCount / 2);
    sort($aToCareAbout, SORT_NUMERIC);
    $median = $aToCareAbout[$middle_index]; // assume an odd # of items

    // Handle the even case by averaging the middle 2 items
    if ($iCount % 2 == 0)
        $median = ($median + $aToCareAbout[$middle_index - 1]) / 2;

    return $median;
}

I don't write PHP but from looking at the online manual for count:

count() may return 0 for a variable that isn't set, but it may also return 0 for a variable that has been initialized with an empty array. Use isset() to test if a variable is set.

But in your case, the function doesn't seem to care whether the array is empty or the variable isn't set -- 0 is returned in both cases. By checking what count returns we could eliminate some of the if branches.

Answer 3

1. While there is nothing wrong with defining your class method as private, I'm going to demonstrate with public for simplicity.
2. I prefer to write @throws in my docblock so that my IDE can remind me if I am not properly catching what is thrown.
3. I recommend type declarations whenever reasonable. The sole incoming argument must be an array and the return value will be numeric. From PHP8, union types are available — in this method, the return may be an integer or a float.
4. Checking for an empty array is as simple as writing a "falsey" check.
5. Throw a LengthException when the array is empty.
6. Sorting the array is an essential part of this process, but even if the array values are strings, sort() will still treat the values numerically. In other words, it is not necessary to explicitly flag the sort with SORT_NUMERIC.
7. After calculating the middle index of the array, if the value is a float, truncate the float to become an integer and early-return the corresponding array value.
8. Otherwise, the middle index is an integer, then the array has an even length and the middle two values must be used in the calculation.
9. In earlier versions of PHP, it used to be acceptable to access array values via a float value as the key. Now Deprecated: Implicit conversion from float [floatValue] to int loses precision is presented if the key is not converted to an integer.
10. The snippet below avoids single-use variable declarations and complies with PSR-12 coding guidelines.

Code: (Demo)

class MyMath
{
    /**
     * @throws Exception
     */
    public function calculateMedian(array $array): int|float
    {
        if (!$array) {
            throw new LengthException('Cannot calculate median because Argument #1 ($array) is empty');
        }
        sort($array);
        $middleIndex = count($array) / 2;
        if (is_float($middleIndex)) {
            return $array[(int) $middleIndex];
        }
        return ($array[$middleIndex] + $array[$middleIndex - 1]) / 2;
    }
}


$math = new MyMath;
var_export($math->calculateMedian([1, 2, 3, 4]));

Answer 4

Personally this is the way that I would build the function:

function calculateMedian($Values)
{
    //Remove array items less than 1
    $Values = array_filter($Values,array($this,"callback"));

    //Sort the array into descending order 1 - ?
    sort($Values, SORT_NUMERIC);

    //Find out the total amount of elements in the array
    $Count = count($Values);

    //Check the amount of remainders to calculate odd/even
    if($Count % 2 == 0)
    {
        return $Values[$Count / 2];
    }

    return (($Values[($Count / 2)] + $Values[($Count / 2) - 1]) / 2);
}

What changes have I made?

• I have used less variables, overwriting the $Values where needed

• Reduced the conditional statements to 1* from 2

• Made the code look more readable and understandable.

• I have however added a callback, which in turn removes the foreach and if statements but a logical check would have to be used in the callback.Tthe callback would simply be a method in your class like so:

    public function callback($value)
    {
        return $value > 0;
    }
  

• Unfortunately as the native function empty is actually a language construct it's not a valid callback; you can however use return !empty($value); within your callback method to also remove other entities such as NULL,FALSE etc.

• This can be removed as stated, and placed outside the function.

*Notes: I would advise you to have some kind of linear array check to make sure the arrays are based on an integer index, as our code assumes they are, a linear check can be done like so:

if(array_keys($Values) !== range(0,($Count-1)))
{
    return null;
}

This would be added after the $Count value has come into play.

Example test that I had used to test it was:

$values = array(
    0,4,7,5,6,9,5,3,2,7,5,6,4,3,7
);
echo calculateMedian($values);

which resulted in the correct answer of 5.