Calling functions with variable functions

Question

I have some PHP code where I use variable functions to call the right function. I need to build a chart array (for example), and the chart array that comes out has a fixed format. But the data that goes into the chart or table comes from varying places in the underlying objects.

An example:

private function buildChartGroups(PropelCollection $groups, $propertyForDisplay) {
    $groupDisplayFunction = "getSum$propertyForDisplay";

    $chart = array();
    foreach ($groups as $group) {
            $chart[] = array('name' => $group->getName(), 
                             'y' => $group->$groupDisplayFunction()
                       );
        }
    }
    return $chart;
}

and then the function gets called something like:

$result = $this->buildChartGroups($groups, "Total");
...elsewhere...
$result = $this->buildChartGroups($groups, "Count");

where $groups is an array of Group objects:

class Group {
   private $name;
   private $sumTotal;
   private $sumCount;

   public function getName() {
       return $this->name;
   }

   public function getSumTotal() {
        return $this->sumTotal;  
   }

   public function getSumCount() {
        return $this->sumCount;
   }
}

This works fine and lets me re-use buildChartGroups() which I would otherwise not be able to do. But using variable functions and passing function names as strings always seems a little dirty, even though I'm sure it has its place.

Is there a way to refactor this or is there some pattern I can use to avoid them?

Answer 1

There are several other ways to solve this, but which one I'd go with depends a lot on the Group class.

• Do you plan to have more than a handful of properties to choose from?
• Do you have control over the Group implementation?
• Is this property-choosing behavior needed anywhere else?

Here's an example that assumes the answers to the above are all "no": use constants to pick the property and provide one-liner public methods to avoid exposing them to callers:

const TOTAL = 1;
const COUNT = 2;

public function buildChartGroupTotals(PropelCollection $groups) {
    return $this->buildChartGroups($groups, self::TOTAL);
}

public function buildChartGroupCounts(PropelCollection $groups) {
    return $this->buildChartGroups($groups, self::COUNT);
}

private function buildChartGroups(PropelCollection $groups, $property) {
    return array_map(function ($group) {
        switch ($property) {
            case self::TOTAL:
                $value = $group->getSumTotal();
                break;
            case self::COUNT:
                $value = $group->getSumCount();
                break;
            default:
                throw new InvalidArgumentException("Invalid property");
        }
        return array(
            'name' => $group->getName(), 
            'y' => $value,
        );
    }, $groups);
}

If you have control over the Group implementation, I would prefer to move the constants and switch into it and add getSumProperty($property).

class Group {
    const TOTAL = 1;
    const COUNT = 2;
    ...
    public function getSumProperty($property) {
        switch ($property) {
            case self::TOTAL:
                return $this->sumTotal;
            case self::COUNT:
                return $this->sumCount;
            default:
                throw new InvalidArgumentException("Invalid property");
        }
    }
}

Now you can pass one of the constants to buildChartGroups which will forward it to getSumProperty and greatly simplify the code at the cost of exposing those constants. You could still provide the property-specific one-liners, though.

private function buildChartGroups(PropelCollection $groups, $property) {
    return array_map(function ($group) {
        return array(
            'name' => $group->getName(), 
            'y' => $group->getSumProperty($property),
        );
    }, $groups);
}

Answer 2

I agree, it does feel dirty to compute the function name like that.

A typical way to do dynamic dispatching is using polymorphism. You could formalize the mechanism for determining which function to call by making some property-extracting objects:

abstract class PropertyExtractor {
    public abstract function getProperty($group);
}

class SumTotalExtractor extends PropertyExtractor {
    public function getProperty($group) {
        return $group->getSumTotal();
    }
}

class SumCountExtractor extends PropertyExtractor {
    public function getProperty($group) {
        return $group->getSumCount();
    }
}

---

Your buildChartGroups() would take one of those extractors as the second parameter:

private function buildChartGroups(PropelCollection $groups, $propertyExtractor) {
    $chart = array();
    foreach ($groups as $group) {
        $chart[] = array('name' => $group->getName(), 
                         'y' => $propertyExtractor->getProperty($group));
    }
    return $chart;
}

---

Here it is in use:

$result = $this->buildChartGroups($groups, new SumTotalExtractor());