Question

This is my callback for my usort()

public function sortProperties($a, $b) {

        $sortA = inflector::camelize(str_replace('-', '_', $this->sortBy));
        $sortB = inflector::camelize(str_replace('-', '_', $this->sortBy));

        $a = Arr::get($a, $sortA);
        $b = Arr::get($b, $sortB);


        if (is_numeric($a) AND is_numeric($b)) {
            return  $a < $b; 
        } else {
            return strcasecmp($a, $b); 
        }


    }

Usually, when I see the first 2 lines in any of my code, it screams to me: refactor! I guess it's because they are identical.

I know I could make a function getCamelized(), but I don't think I'd use it again outside of this.

Is there a way to turn those 4 lines into 2? Could func_get_args() or array_walk() help me here?

Also, is there anything wrong about this sorting function?

Was it helpful?

Solution

Is there a way to turn those 4 lines into 2?

    $sortA = $sortB = inflector::camelize(str_replace('-', '_', $this->sortBy));

And for other two lines:

    list($a, $b) = array(Arr::get($a, $sortA), Arr::get($b, $sortB));

As for sort, it seems to be fine at least to me.

OTHER TIPS

$sortA == $sortB so that part is just duplication. Calculate $sortA once wherever you set $this->sortBy. The Arr::get lines you're stuck with. The return $a < $b; seems wrong, you should be returning a -ve, 0, +ve number.

...
function setSortBy($sortBy) {
    $this->sortBy = $sortBy;
    $this->sortByCam = inflector::camelize(str_replace('-', '_', $sortBy));
}
....

public function sortProperties($a, $b) {

    $a = Arr::get($a, $this->sortByCam);
    $b = Arr::get($b, $this->sortByCam);

    if (is_numeric($a) && is_numeric($b)) {
        return $a - $b;
    } else {
        return strcasecmp($a, $b); 
    }

}

Something like that. The main idea to get the camelizing part out of the loop.

Be aware that strcasecmp will return an int (1, 0, or -1) and < will return a boolean. You really need to be using one or the other. Also note that strnatcasecmp will probably give you the behavior you want for both numbers and strings so try this one:

public function sortProperties($a, $b) {
  $aInflected = Arr::get($a, $sort = inflector::camelize(str_replace('-', '_', $this->sortBy)));
  return strcasecmp($aInflected, Arr::get($b, $sort));
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top