Introducing Object Calisthenics

The concept of object callisthenics was first introduced by Jeff Bay. The concept was ported to PHP by Rafael Dohms and Guilherme Blanco. They include programming exercises that allow a developer to improve and cleanup code, adhering to the mentioned standards like SOLID and DRY. My blog is not meant as a complete write-up of all of the details - much has been written already about them - but simply a brief introduction.

For me, the main reason to follow callisthenics is not to live strictly by them, but instead, follow the rules when needed and use them as a solid way to refactor my code. The more I follow them, the better my code becomes. However, when I don't follow these rules, I don't feel like shooting myself in the foot. At least, I've given my code a good second thought.

1. Only One Level Of Indentation Per Method

Each class method might have zero or more indents in the code. Each if or foreach loop creates a new indent, making the method harder and harder to read. The exercise is to bring the number of indents down to one. Following from this, if you have an if within a foreach actually the if code should be moved to a second class method (of course properly named).

In practice, this calisthenic allows for a lot of cleaning up, though it is not always practical: Some of my methods are more readable when they have 2 or 3 indents, instead of spreading the code across multiple methods (which might lead to too many class methods). However, 10 indents definitely is evil.

2. Don’t Use The ELSE Keyword

When you're building a lot of if structures, often you can also include a lot of else structures. However, the if-else structure is often a bit confusing. Let's take a look at the following segment because it does well better than just theory:

public function isAllowed()
{
    if ($this->_allowed) {
        return true;
    } else {
        return false;
    }
}

While the code is really basic, it is messy. First of all, the else can be omitted entirely:

public function isAllowed()
{
    if ($this->_allowed) {
        return true;
    }
return false;

}

However, the default of the method is now to return false, while actually isAllowed() might suggest that the default is true. If the default would be false, I would expect a method name like isAllowedAnyway(). To make the workflow of the method more understandable, we can reorder the method as such:

public function isAllowed()
{
    if ($this->_allowed === false) {
        return false;
    }
return true;

}

The difference between the three segments is only small, but it shows the approach of an early return (in this case, the return false happens as soon as the condition occurs. It also shows that removing an else is often very easy. If it is not, your method is most likely too complex. Looking at Magento code, this is often forgotten: There are still a lot of Magento classes (in both Magento 1 and Magento 2) that contain methods with a lot of elses.

Also note that this little example might have been cleaned up even further by using a ternary operator. However, this code segment was more about demonstrating the early return.

3. Wrap All Primitives And Strings

If you have a product with a certain price, the product class might contain the price as a direct double. However, this also requires that the product class contains additional methods like showPrice() with currency support and money formatting rules. These methods might be moved to a product helper class, but that's a code smell. Instead, the price should be contained in its own class. This is already true for both Magento 1 and Magento 2.

However, there is still room for improvement. Magento 2 deals with translations using a global function __(), while actually the procedure of translation could be dealt with using a class of some kind. Likewise in your own code, you might be using database queries by manually passing them through to a PDO adapter - it would be cleaner to use a Query class for that.

4. First Class Collections

When there is a class that has only the purpose to contain a collection of objects, that class should not contain any other member variables except for the collection itself. With Magento 1, you can map this rule directly to the usage of Magento collections. For instance, a product collection contains a listing of product objects.

However, with Magento, a collection also serves as a kind of service to Resource Models - dealing with database stuff. Collections are not just a container of objects. With Magento 2, this issue is solved by using repositories instead. So to adhere to this calisthenic in Magento 2, collections should be swapped for repositories.

5. One Dot Per Line

Well, a bit misleading because object callisthenics came from Java, where the dot is actually used for connecting objects to their objects. With PHP we use arrows, so actually, the rule should be One Arrow Per Line ($object->getName()). Take for instance the following code:

$productSku = Mage::getModel('catalog/product')
    ->getCollection()
    ->addAttributeToSelect('sku')
    ->getFirstItem()
    ->getSku()
;

Ok, all of the arrows are each on its own line - correct. But that's not what the rule is about. What you see here is method chaining gone wild. Each method returns its own object, different from the other objects, while the method chaining suggests they are all the same object. Instead, this code should be cleaned up by being explicit about what is returned by which method:

$productModel = Mage::getModel('catalog/product');
$productCollection = $productModel->getCollection()
    ->addAttributeToSelect('sku')
;
$product = $productCollection->getFirstItem();
$productSku = $product->getSku();

The code is more elaborate and perhaps costs even more memory, but at least it is understandable. If a method returns a new object, it is stored in a new variable. The methods getCollection() and addAttributeToSelect() return the same object (the collection) so can be grouped together.

Cleaning up the code here could be done by applying the Law of Demeter.

6. Don’t Abbreviate

Simple rule. Don't abbreviate. For instance, $product should not be $prdct and $orderItem should not be $i. Shorter naming should not be used as some weird kind of way to save memory - we are not living in the 70s anymore. Instead, names should be as descriptive as possible. This also counts for general object names that actually contain specific objects. It is better to name a product collection $productCollection than $collection.

7. Keep All Entities Small

While some of the previous callisthenics require you to expand your code across more methods and lines, this one asks you to undo that. Keep all entities small, where entities can refer to files, classes and methods. Each class should have its own file, and one file should not contain more than one class.

If the number of lines within a method does not fit on the screen, spread it out over multiple methods. If the number of methods grows, create more classes. The official calisthenic says to keep classes limited to 50-100 lines and looking at the Magento code base I see some room for improvement. I always use this rule in the following manner: If I don't quickly understand the nature of the entity simply by scrolling through it, the entity has become too complex.

8. No Classes With More Than Two Instance Variables

Things get tougher. A class should have a limited number of instance variables - ideally only none, one or two. With product classes, this fails soon, because products have more than 2 attributes (ID, name, SKU, damn!). But to remind you, it is not about the strict follow-up of the rule, it is about the spiritual experience when you're trying to live up to the rule. Having too many instance variables creates classes that are more difficult to understand. The fewer the instance variables, the less complex the class becomes.

Perhaps it is also wise to differ between data models and service models. Data models are there to encapsulate data. An instance variable is actually simply a representation of a data element, so having multiple instance variables simply reflects the data. However, a service module could do something with that data model. For instance, a price calculator could reuse a product object (data model), but there should not be much more than that product within that calculator.

9. No Getters/Setters/Properties

This one might be a bit disturbing. No getters and settings? About 90% of all class methods in Magento are getters and setters. Does Magento need to be rewritten entirely? There is no magic solution here: If a class indeed needs specific getters and setters, that's what the class needs. However, often, getters and setters are used to use the class as a data model, without actually asking why these getters and setters are needed.

Instead of asking the class for a specific thing, you should tell the class what to do. For instance, a pricing model could have the ability to calculate tax using a method like getPriceInclTax() while a more explicit method name like calculatePriceInclTax() would be more explicit about what the method needs to do. With getPriceInclTax() it is unclear whether the price already includes a tax or not, whether there is a separate variable $price_incl_tax or not, if the calculation is done by the method itself or whether it is deferred to another method. Not using getters and setters forces you to rethink the relationship between classes.

10. ... document?

While the original callisthenics of Jeff counted only 9 rules, sometimes a tenth rule is added to it: Document your code. I agree with this partially, after having read various other books like the book Clean Code of Robert C. Martin. So for me, this translates into documenting your code if it is not yet self-explanatory. A DocBlock stating that a method __constructor() is a constructor is pointless. Likewise a comment This method returns the product price is pointless if the method name is getProductPrice(). There are still code structures that might need documenting. However, if the code needs documenting, there is probably a code smell - and the other 9 rules need to be applied.

Hope you find these callisthenics useful. I'm already rewriting my Magento code, by following these simple rules!