The switch (get_class($item))
part of your code is an incorrect implementation of the OOP concept called polymorphism.
The correct way to do it is to define an interface that declares a method (let's name it updateAmounts()
) then implement the interface in all classes you can have as $item
in the inner loop. Each implementation of the updateAmounts()
method contains the code from corresponding case
of the switch
statement.
Something like this:
// This class is a Value Object.
// It doesn't need behaviour and it's OK to have public properties
class Amounts
{
public $revenue = 0;
public $fees = 0;
}
interface HasAmounts
{
/**
* Update the revenue and the fees in the passed Amounts object
* using the data stored in this object.
*/
public function updateAmounts(Amounts $a);
}
class PrincipalItem implements HasAmounts
{
public function updateAmounts(Amounts $a)
{
$a->revenue += $this->getAmountAfterTax() * $this->quantity;
}
}
class PriceItem implements HasAmounts
{
public function updateAmounts(Amounts $a)
{
$a->revenue += $this->getAmountAfterTax();
}
}
class FeeItem implements HasAmounts
{
public function updateAmounts(Amounts $a)
{
$a->fees += $this->amount;
}
}
Now the nested loops look like this:
$amounts = new Amounts();
foreach ($this->orders as $order) { // OrderObject
foreach ($order->items as $itemGroup) { // ItemPrice, ItemFees
foreach ($itemGroup as $item) { // PrincipalItem, PriceItem, FeeItem
$item->updateAmounts($amounts);
}
}
}
echo('Revenue: '.$amounts->revenue."\n");
echo('Fees: '.$amounts->fees."\n");
If the classes PrincipalItem
, PriceItem
and FeeItem
already extend the same base class (or have a common ancestor) then the method can be added (as abstract
or with an empty implementation) to this class (and the interface is not needed any more).
After the above transformation of the code, do you still want to find a way to reuse the nested foreach
block?
The foreach
statements don't look worth reusing now. This happens because the code that is worth reusing was moved where it belongs; the code should stay together with the data it handles, in the class.
Further refactoring can be operated on the code by moving each foreach
in the class that contains the data it operates on.
For example:
class OrderObject implements HasAmounts
{
public function updateAmounts(Amounts $a)
{
foreach ($this->items as $itemGroup) { // ItemPrice, ItemFees
foreach ($itemGroup as $item) { // PrincipalItem, PriceItem, FeeItem
$item->updateAmounts($amounts);
}
}
}
}
If $itemGroup
is an object of some class (or more) then the inner foreach
can be moved to each of these classes (in its implementation of the updateAmounts()
method) and this way the code stays in the same class with the data it processes (it's called encapsulation and it is another important property of the OOP code).
Now the calling code now looks like this:
$amounts = new Amounts();
foreach ($this->orders as $order) { // OrderObject
$order->updateAmounts($amounts);
}
echo('Revenue: '.$amounts->revenue."\n");
echo('Fees: '.$amounts->fees."\n");
Look, ma! No more nested foreach
loops
I'm afraid I tore apart your pack of nested foreach
loops and there is nothing to reuse from it.
But wait! Your classes now contain behaviour (they probably used to be just amorphous containers of data) and this is better than writing all-purpose code (as it seems it was your goal of reusing the foreach
loops). Because this is what OOP is supposed to be: data and code packed together.