0

I've got a class which stores two data series like so. I need to do some complicated processing on each of these arrays, but for now we'll just get the average of each data series. I could either do this:

class DataMangler
{
    private array $dataArrayA = [];
    private array $dataArrayB = [];

    // stand in for complicated calculation
    private static function getAverage(array $dataArray): ?float
    {
        $sum = array_sum($dataArray);
        $count = count($dataArray);
        if (0 === $count) {
            return null;
        }
        return $sum / $count;
    }

    public function getAAverage(): ?float
    {
        return self::getAverage($this->dataArrayA);
    }

    public function getABverage(): ?float
    {
        return self::getAverage($this->dataArrayB);
    }
}

and then I can call each method individually.

Or I could add getters for each data array, and the caller has to pass them into the (now public) average calculation:

class DataMangler
{
    private array $dataArrayA = [];
    private array $dataArrayB = [];

    // stand in for complicated calculation
    public static function getAverage(array $dataArray): ?float
    {
        $sum = array_sum($dataArray);
        $count = count($dataArray);
        if (0 === $count) {
            return null;
        }
        return $sum / $count;
    }

    public function getA(): array
    {
        return $this->dataArrayA;
    }

    public function getB(): array
    {
        return $this->dataArrayB;
    }
}
$dataMangler->getAverage($dataMangler->getA());

Is there a rule of thumb for the better pattern?


(While writing this, I thought of a third option which is to make the dataArrays be fully fledged objects, which contain the logic for the complicated calculation. It still raises the question of getters for each DataArray object a la $dataMangler->getA()->getAverage() or add wrappers around the average so it is still $dataMangler->getAAverage())

2
  • 1
    Your third option looks good, but it raises the question why you then need a "datamangler" class at all. Another question of mine would bem if in case you decide to stay with this "datamangler", is there any other code in the system outside "datamangler" which would call getA() and getB()?
    – Doc Brown
    Sep 22, 2021 at 13:59
  • @DocBrown Good point - I suppose the example was too simple. The DataMangler is still required to mangle the two objects in one method (e.g. find average of differences for each index). And let's say no other class needs access to A and B in their raw values, only the mangled values, so getters wouldn't be present. Sep 22, 2021 at 16:06

2 Answers 2

1

Generally encapsulation would suggest the first. In the second, not only does the caller have to pass in the array, but suddenly they can modify it, pass in their own array, etc. And you've encoded the array type in the public API, so if you wanted to change it to the third option, you can't without changing all the call sites.

0

You're designing part of the internal implementation and in part also considering the interface offered a consuming client.  It is a bit of an iterative process.  But, you won't really know what's better until you have a real consumer interacting with this code.

We should prioritize the consumption (of code) by interface — whether it works simply & naturally for the consuming client or is awkward to use, then after that, the internal implementation details.  Ideally, the consuming client has to do as little as possible, and doesn't have remember multiple things or anything unnecessary to do to use the interface in the simplest and most straightforward way.  It would even be worth making the internal implementation more complex if that truly simplifies the consuming client's job in usage.

This argues in favor of the first design over the second, since in the second design the client has to extract a field member and pass it to a static method, rather than the simpler calling a method on the object.

Try to identify the right abstractions, and the right number of abstractions you need to model something sensible for the consuming client.  So, what job is the consuming client trying to do, and then what interfaces and objects do you want to offer the consuming client?  Then design the code for that interaction (and iterate to upgrade the interface/abstractions as needed).

It is unclear why two arrays are being paired together, but there's not enough information to suggest an alternative.  Objects are bindings of things that logically belong together, and this should be evident, for example, in the lifetime of internal state (should all be the same lifetime, if one array is populated in construction, but the other's is delayed it suggests a mismatch), and, should also be evident in usage e.g. methods.  Packaging two arrays together and offering separate methods that average one or the other seems like the binding of the two arrays together into one object isn't of particular value, as far as we can see from what's shown.

Also as you mention, perhaps there is missing abstraction — that of a single array, but that is unclear from what we're seeing.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.