从对象中删除太多IF

Question

Edit:

I should have mentioned that I wanted it to be more Object Oriented. And I don't think my code here is anywhere near OO and neither is using switches, is it?

OP:

First of all, in my below example I am working with dutch units, so the calculations might seem off, but you'll get the idea.

Basically, what I have is a grocery list with products. In my database I store prices in "price by piece" or "price by pound", for example. So in order for calculating the total price of each product, based on the amount selected, I am working with the below class.

Example:

In my grocerylist I have a few products, and behind that product is a text field and a dropdown. In the textfield I can enter the amount I want to have, and in the dropdown I select if it needs to be ounces, pounds, and so on. Based upon those values, and the initial price from my database (price per piece and so on), I can calculate the total price for each product.

class Calculation
{
    protected $price;
    protected $amount;
    protected $unit;

public function __construct($price, $amount, $unit)
{
    $this->price = $price;
    $this->amount = $amount;
    $this->unit = $unit;
}

public function calculate()
{
    if($this->unit === 'ounce')
    {
        return $this->formatOunce();
    }
    if($this->unit === 'pound')
    {
        return $this->formatPound();
    }
        return $this->formatOne();
}

public function formatOne()
{
    return $this->price * $this->amount / 100;
}

public function formatOunce()
{
    return $this->price / 1000 * $this->amount / 100;
}

public function formatPound()
{
    return $this->price / 1000 * 500 * $this->amount / 100;
}

}

The problem I have is this:

public function calculate()
{
    if($this->unit === 'ounce')
    {
        return $this->formatOunce();
    }
    if($this->unit === 'pound')
    {
        return $this->formatPound();
    }
        return $this->formatOne();
}

How would I change the above code in order for it to be good OO? Do I use a Repository or an Interface for that? Or can I do that within this particular class to keep it simple? I feel there is way too many IF's.

Answer 1

A OO oriented approach (as requested after editing the question):

You make a base class that handles the output total(). It calls a protected method that does the calculation. calculate():

class Item
{
    protected $price;
    protected $amount;

    public function __construct($price, $amount)
    {
        $this->price = $price;
        $this->amount = $amount;
    }

    protected function calculate()
    {
        return $this->price * $this->amount;
    }

    public function total($format = true)
    {
        if ($format) {
            return number_format($this->calculate(), 2);
        }
        return $this->calculate();
    }
}

Now you can extend you base item with a Pound version of the item. The Pound version will override the calculate() method because the calculation is done differently.

class PoundItem extends Item
{
    protected function calculate($format = true)
    {
        return $this->price / 1000 * 500 * $this->amount / 100;
    }
}

To produce your objects you'll need either a constructor method or what is called a factory to produce them. Here is a factory class. It could just as well have been implemented on your basket class.

class ItemFactory
{
    static public function create($price, $amount, $type)
    {
        // this could be implemented in numerous ways
        // it could even just be method on your basket
        $class = $type . "Item";
        return new $class($price, $amount);
    }
}

Creating a new item of the Pound type:

$a = ItemFactory::create(49.99, 25, "Pound");

Since PoundItem is also an Item you can use the total() method. But since we've changed the implementation of calculate() it now calculates for pounds.

echo $a->total();