9 rules for writing (better?) code

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.

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

Several possible solutions:

// ✅ 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 :

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 :

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

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

    return $urlToRedirect;
}

// ✅ 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

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');
            }
        }
    }
}

// ✅ 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.

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

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

    //...
}

// 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 :

// 🚫 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);
        }
    }
}

// ✅ 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 :

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

// ✅ 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.

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);
    }

    // ....
}

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

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()
);

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

$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.

🚫 $m->send($contact);

✅ $mailer->send($contact)

🚫 $cmpMng->getById($id);

✅ $companyManager->getById($contact)

🚫 $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

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);
    }
}

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”

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;
    }
}

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 for more PHP tips.

Comments

[Fatemeh Paghar]

The article underscores the importance of maintaining consistency across development teams by following standardized coding practices. Consistent coding styles and adherence to the outlined rules contribute to better collaboration and understanding among team members. When everyone follows the same set of principles, it becomes easier to review, contribute, and maintain code collectively. This consistency not only enhances code quality but also fosters a cohesive and efficient development environment. Emphasizing team-wide adherence to these principles can lead to a smoother and more productive coding experience for everyone involved. 👥💻

[Jimmy Klein]

I agree with you 👍 It is important to follow coding standards across a team.

But it can be complicated to go to these level of details. Each developers has its own habits (he can share some of them with the other members of his team).

[Marat Latypov]

Great article!
Sadly I have to use setters/getters every single day. In Symfony world it's highly hard to avoid it

[Jimmy Klein]

Thank you 🙏 !
Have you ever try to don't generate getter and setter in your doctrine entity ?
I would say that it is not mandatory.
But it is a difficult rule, we are so used to have getter and setter.

[Marat Latypov]

Yes, I know Doctrine doesn't require it. I'm pretty sure some Symfony package does Don't remember exactly... Forms maybe ...

I have to try it on some home project