patternphpMajor
Calculate a median
Viewed 0 times
mediancalculatestackoverflow
Problem
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.
= 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;
}
}Solution
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
This way we have generally available all purpose function, with naming consistent with core php functions.
And your method would look like
Either within
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)Code Snippets
/**
* 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;
}/**
* 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
)
);
}Context
StackExchange Code Review Q#220, answer score: 23
Revisions (0)
No revisions yet.