9 rules for writing (better?) code
Tags: webdev, php, codequality
Published: 2024-02-28 11:48:05

Like many developers, I like learning new things. And one thing that interests me a lot is code quality and how to write code.

In order to practice in different areas, I quite often do Katas to which I add constraints. And a while ago I discovered **Calisthenics Object**.

Behind this name lie 9 rules to follow when writing code. And suffice to say that some of these rules are quite contrary to everything we learn at school or in the various tutorials and online training courses.

The goal is to have code that is more **maintainable**, **more readable** and **more easily testable**.

> Code is read much more often than it is written

_The following rules are not to be taken literally. The principle is to experiment with them on small personal projects or during Kata code in order to see what would be interesting to apply in a real situation._

---

## 1. Only one level of indentation per method

This makes the code easier to read and maintain.

```php
// 🚫 2 levels of indentation
public function notify(array $contacts)
{
    foreach ($contacts as $contact) {
        if ($contact->isEnabled()) {
            $this->mailer->send($contact);
        }
    }
}
```

Several possible solutions:
```php
// ✅ Extracting indented code in another method
public function notify(array $contacts)
{
    foreach ($contacts as $contact) {
        $this->notifyContact($contact)
    }
}

private function notifyContact(Contact $contact)
{
    if ($contact->isEnabled()) {
        $this->mailer->send($contact);
    }
}

// ---------------------------------------------------

// ✅ We filter the list of contacts before the loop 
public function notify(array $contacts)
{
    $enabledContacts = array_filter(
      $contacts,
      fn($contact) => $contact->isEnabled()
    );
  
    foreach ($enabledContacts as $contact) {
        $this->mailer->send($contact);
    }
}

// ---------------------------------------------------

// ✅ We ask the caller to send us directly the list of
// activated contacts
public function notify(array $enabledContacts)
{
    foreach ($enabledContacts as $contact) {
        $this->mailer->send($contact);
    }
}
```

---

## 2. Do not use the `else` keyword

Using `else` forces us to read nested code (with more levels of indentation) whereas in most cases we can do without it.

### First solution: use of early return

Before : 
```php
class ItemManager
{
    private $cache;
    private $repository;

    public function __construct($cache, $repository)
    {
        $this->cache = $cache;
        $this->repository = $repository;
    }

    public function get($id)
    {
        if (!$this->cache->has($id)) {
            $item = $this->repository->get($id);
            $this->cache->set($id, $item);
        } else {
            $item = $this->cache->get($id);
        }

        return $item;
    }
}
```

After :
```php
class ItemManager
{
    private $cache;
    private $repository;

    public function __construct($cache, $repository)
    {
        $this->cache = $cache;
        $this->repository = $repository;
    }

    // ✅ Early returns
    public function get($id)
    {
        if ($this->cache->has($id)) {
            return $this->cache->get($id);
        }

        $item = $this->repository->get($id);
        $this->cache->set($id, $item);

        return $item;
    }
}
```

### Upstream initialization
```php
public function redirect(User $user)
{
    if ($user->isAuthenticate()) {
        $urlToRedirect = '/dashboard';
    } else {
        $urlToRedirect = '/login';
    }

    return $urlToRedirect;
}
```

```php
// ✅ Initialization ahead of default value
// valid if initialization is inexpensive
public function redirect(User $user)
{
    $urlToRedirect = '/login';
    if ($user->isAuthenticate()) {
        $urlToRedirect = '/dashboard';
    }

    return $urlToRedirect;
}
```

### Use of the Fail Fast principle
```php
class MyListener
{
    public function onDelete(Event $event)
    {
        if ($event->getType() === 'OBJECT_DELETE' 
            && $event->getObject instanceOf MyEntity) {
            $this->cache->invalidate($event->getObject());
        } else {
            if ($event->getType() !== 'OBJECT_DELETE') {
                throw new \Exception('Invalid event type');
            } else {
                throw new \Exception('Invalid object instance');
            }
        }
    }
}
```

```php
// ✅ Use of the Fail Fast principle: 
// we immediately test error cases
class MyListener
{
    public function onDelete(Event $event)
    {
        if ($event->getType() !== 'OBJECT_DELETE') {
            throw new \Exception('Invalid event type');
        }

        $myEntity = $event->getObject();
        if (!$myEntity instanceOf MyEntity) {
            throw new \Exception('Invalid object instance');
        }

        $this->cache->invalidate(myEntity);
    }
}

```

---

## 3. Wrap all primitive types in objects
(especially those who have particular behaviors)

Benefits :
* encapsulation of processing
* type hinting
* validation of parameters upstream.

```php
public function fizzBuzz(int $integer)
{
    if ($integer <= 0) {
        throw new \Exception('Only positive integer is handled');
    }

    if ($integer%15 === 0) {
        return 'FizzBuzz';
    }

    //...
}
```

```php
// Replacing int with a PositiveInteger object
public function fizzBuzz(PositiveInteger $integer)
{
    // ✅ No more validation testing of the input parameter
    if ($integer->isMultipleOf(15)) {
        return 'FizzBuzz';
    }

    // ...
}

// Using a Value Object
class PositiveInteger
{
    private $value;

    public function __construct(int $integer)
    {
      	// ✅ The integer validation test is done directly here
        if ($integer <= 0) {
            throw new \Exception('Only positive integer is handled');
        }

        $this->value = $integer;
    }

    // ✅ You can even add functions related to this object
    public function isMultipleOf(int $multiple)
    {
        return $this->valueinteger%$multiple === 0;
    }
}
```

Other example :
```php
// 🚫 Passing a table does not allow us to be sure 
// of the content and requires us to do additional tests
public function notify(array $enabledContacts)
{
    foreach ($contacts as $contact) {
        if ($contact->isEnabled()) {
            $this->mailer->send($contact);
        }
    }
}
```

```php
// ✅ Here we directly pass an object containing only activated contacts. 
// We are therefore assured of only having active contacts
public function notify(EnabledContacts $enabledContacts)
{
    foreach ($enabledContacts as $contact) {
        $this->mailer->send($contact);
    }
}

class EnabledContacts implements \Iterator 
{
    private $contacts;

    public function __construct(array $contacts)
    (
        // ✅ We only keep active contacts
        $this->contacts = array_filter(
          $contacts,
          fn(Contact $contact) => $contact->isEnabled()
        );
    )

    // ... définition des méthode de l'interface \Iterator
}
```

Other example :
```php
// 🚫 Two parameters are strongly linked
public function findAll(int $start, int $end)
{
  // paginated data recovery in database
}
```

```php
// ✅ We group two attributes in a single class
public function findAll(Pagination $pagination)
{
  $start = $pagination->getStart();
  $end = $pagination->getEnd();
  
  ...// paginated data recovery in database
}
```

---

## 4. First Class Collections: a class that contains an array as an attribute must not contain any other attribute

The code related to this collection is now encapsulated in its own class.

```php
class Newsletter
{ 
    private int $id;
    private string $title;
  
    // 🚫 The object already contains two attributes, 
    // so it cannot contain an array. 
    // You have to encapsulate it in an object
    private array $subscriberCollection;

    public function getNumberOfSubscriberWhoOpen()
    {
        $subscribersWhoOpen = array_filter(
          $this->subscriberCollection,
          fn($subscriber) => $subscriber->hasOpenNewsletter()
        );

        return \count($subscriberWhoOpen);
    }

    // ....
}
```

```php
class Newsletter
{
    private int $id;
    private string $title;
  
    // ✅ The array is now encapsulated in its own class
    private SubscriberCollection $subscriberCollection;

    public function getNumberOfSubscriberWhoOpen()
    {
        return $this->subscriberCollection
            ->getSubscriberWhoOpen()
            ->count();
    }

    // ....
}

class SubscriberCollection 
{
    private array $subscriberCollection;

    // ✅ We can declare here "business" methods 
    // linked to subscribers
    public function getSubscriberWhoOpen()
    {
      	$subscribersWhoOpen = array_filter(
          $this->subscriberCollection,
          fn($subscriber) => $subscriber->hasOpenNewsletter()
        );
      
        return new static($subscribersWhoOpen);
    }

    // ...
}
```

---

## 5. Only one `->` per line (except for Fluent interface)

The goal here is not to have nicely formatted code, but to follow Demeter's law: "Speak only to your immediate friends."

Obviously `$this->myAttribute` does not count in the count

```php
class User
{
    private ?Identity $identity;

    public function getIdentity(): ?Identity
    {
        return $this->identity;
    }
}

class Identity
{
    private string $firstName;
    private string $lastName;

    public function getFirstName(): string
    {
        return $this->firstName;
    }

    public function getLastName(): string
    {
        return $this->lastName;
    }
}

$user = new User();
$fullName = sprintf(
  '%s %s',
  // 🚫 Non-compliance with the law of demeter
  // 🚫 getIdentity() could return `null` and this would 
  // generate an error
  $user->getIdentity()->getFirstName(), 
  $user->getIdentity()->getLastName()
);
```

```php
class User
{
    private ?Identity $identity;

    public function getFullName(): string
    {
      if ($this->identity === null) {
      	return 'John Doe';
      }
      
      return sprintf(
        '%s %s',
        // The original rule applies for example to java 
        // where the keyword “this” does not need to be 
        // specified in classes.
        // We therefore do not count the first here `->` 
        // because in PHP $this is mandatory in classes
        // to use an attribute
        $this->identity->getFirstName(),
        $this->identity->getLastName()
      );
    }
}

class Identity
{
    private string $firstName;
    private string $lastName;

    public function getFirstName(): string
    {
        return $this->firstName;
    }

    public function getLastName(): string
    {
        return $this->lastName;
    }
}

$user = new User();
// ✅ Respect for the law of Demeter
// ✅ More error handling here
$fullName = $user->getFullName();
```

This rule does not apply to fluent interfaces such as query builders for example
```php
$query = $queryBuilder
  ->select('user')
  ->where('user.id = :id')
  ->setParameter('id', 1);
  ->getQuery()
;
```

---

## 6. Not using abbreviations

One of the simplest rules to apply and above all to **apply immediately**!

* Better comprehension
* Better maintainability
* If you can't name: the function does too many things, the content of the variable is unclear, etc.

```php
🚫 $m->send($contact);

✅ $mailer->send($contact)
```

```php
🚫 $cmpMng->getById($id);

✅ $companyManager->getById($contact)
```

```php
🚫 $translator->trans($input);

✅ $translator->translate($input)
```

---

## 7. Keep all entities small (classes, methods, packages/namespaces)

🔥 Constraints:
* Maximum 10 methods per class
* Maximum 50 lines per class
* Maximum 10 classes per namespace

✅ Objective:
* Limit class responsibilities
* Facilitate the maintainability of classes and methods
* Have a consistent set of classes (namespace)

---

## 8. Classes should not contain more than five instance variables

* Fewer dependencies
* So easier to mock for unit tests

Example here with the limit at 2

```php
class EntityManager
{
    // 🚫 4 attributes
    private EntityRepository $entityRepository;
    private LoggerInterface $logger;
    private MiddlewareInterface $middleware;
    private NotificationService $notificationService;

    public function update(Entity $entity)
    {
        $this->entityRepository->update($entity);

        // 🚫 These three acions could very well be 
        // relocated in order to avoid overloading 
        // this method and to facilitate 
        // the addition of other actions later.
        $this->logger->debug($entity);
        $this->middleware->sendMessage($entity);
        $this->notificationService->notify($entity);
    }
}
```

```php
class EntityManager
{
    // ✅ Fewer dependencies
    // ✅ So easier to mock for unit tests
    private EntityRepository $entityRepository;
    private EventDispatcher $eventDispatcher;

    public function update(Entity $entity)
    {
        $this->entityRepository->update($entity);

        // ✅ It will be very easy to add another 
        // processing by adding a listener to this event
        $this->eventDispatcher->dispatch(Events::ENTITY_UPDATE, $entity);
    }
}

// ✅ Processing has been relocated to 3 separate listeners
// ✅ Small and easily testable classes
class EntityToLog
{
    private LoggerInterface $logger;

    public function onUpdate(Entity $entity)
    {
        $this->logger->debug($entity);
    }
}

class EntityToMiddleware
{
    private MiddlewareInterface $middleware;

    public function onUpdate(Entity $entity)
    {
        $this->middleware->sendMessage($entity);
    }
}

class EntityNotification
{
    private NotificationService $notificationService;

    public function onUpdate(Entity $entity)
    {
        $this->notificationService->notify($entity);
    }
}
```

---

## 9. No getter / setter
* Encapsulation of actions
* Allows you to think on the principle “Tell, don’t ask”

```php
class Game
{
    private Score $score;

    public function diceRoll(int $score): void
    {
        $actualScore = $this->score->getScore();
        // 🚫 We modify its value outside the object 
        // and then “force” the result on it.
        $newScore = $actualScore + $score;
        $this->score->setScore($newScore);
    }
}

class Score 
{
    private int $score;

    public function getScore(): int
    {
        return $this->score;
    }

    public function setScore(int $score): void
    {
        $this->score = $score;
    }
}
```

```php
class Game
{
    private Score $score;

    public function diceRoll(Score $score): void
    {
        $this->score->addScore($score);
    }
}


class Score 
{
    private int $score;

    public function addScore(Score $score): void
    {
      	// ✅ We define here the score addition logic
        $this->score += $score->score;
    }
}
```

🗣️ Tell me in the comments if you already apply some of these rules and if you will try some of them.

---

## Thank you for reading, and let's stay in touch !
If you liked this article, please share. Join me also on [Twitter/X](https://twitter.com/enjimmyklein) for more PHP tips.