Deep Simplicity - Rule 9

So this is the last rule in the Deep Simplicity series. This time we will be talking about rule #9: Don't use Getter/Setters/Properties. At first, I was confused about the rule because the main advantage of OOP is that you have the data and the methods that work on that data in the same place. I have often complained about the ridiculous idea that you should just blanket generate getters and setters for every single property. This leads to copy and paste errors and in my opinion a class that is totally bloated with useless methods.

The basic idea of Rule 9 is that if you just give blanket access to your properties then you are allowing the client code to ignore encapsulation and in doing so you are allowing operations on your object's dirty bits to take place outside of your class. As usual, allowing this to take place is not a good idea. Take a look at this code:

class Product
{
    private $price;
    private $taxable;

    public function getPrice()
    {
        return $this->price;
    }

    public function getTaxable()
    {
        return $this->taxable;
    }
}


class Order
{
    private $taxRate;
    private $products;

    public function getTotal()
    {
        $total = 0;
        foreach ($this->products as $product) {
            if ($product->getTaxable()) {
                $priceWithTax = $product->price * $this->taxRate;
            } else {
                $priceWithTax = $product->price;
            }

            $total += $priceWithTax;
        }
        return $total;
    }
}

The Order::getTotal knows way too much about the product object. What happens if there are other factors in determining the price after taxes? Are you going to put all of that logic into the getTotal method as well? What happens if you want to calculate the price after taxes without having an order?

Code duplication results in multiple places for possible bugs if the spec changes, you have to "remember" where those calculations are done in order to update them all. Encapsulation solves this problem by making one and only one logical place for functionality to reside. It's hard to remember all of the places that code should change if you have never seen it before. Here is the same functionality with no access to the Product's private parts:

class Product
{
    private $price;
    private $taxable;

    public function priceAfterTaxes($taxRate)
    {
        if ($this->taxable) {
            return $this->price * $taxRate;
        }

        return $this->price;
    }
}


class Order
{
    private $taxRate;
    private $products;

    public function getTotal()
    {
        $total = 0;
        foreach ($this->products as $product) {
            $total += $product->priceAfterTaxes($this->taxRate);
        }
        return $total;
    }
}

Wow, not a single accessor in sight. Once again the code we end up with looks much cleaner than the original. The semantic intent is clear and more importantly, this new method is much more likely to be reused.

These rules are meant to be a means to an end. You can always come up with edge cases and exceptions to the rules where you find that the code you end up with is maybe not as good as it could have been. The point is not that you follow these rules to the letter or die trying; the point is that by thinking about these rules as you write code you will end up with something that is far better than you would have otherwise done. I hope that the examples I have given have shed some light on how simple and understandable your code can become with a little more effort on your part. If you are interested in more of the great ideas from Thoughtworks, make sure you grab The Thoughtworks Anthology.

blogroll

social