Help with optimising calls to the usort function in PHP

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?

Answer 1

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.

Answer 2

$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.

Answer 3

“The First Rule of Program Optimization: Don't do it. The Second Rule of Program Optimization (for experts only!): Don't do it yet.” - Michael A. Jackson

There is also another rule that says don't fix what is not broken. Think about it; it's just one line of code, the time you spent posting this question, reading the answer and all the comments could have been used to write a large chunk of code for a different task in your project.

Answer 4

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));
}