How to write readable code?
Tags: beginners, php
Published: 2020-07-23 23:52:23

---
title: How to write readable code?
published: true
description: Why do we write unreadable code? With Examples ...
tags: beginners, php
---

first published here: http://suckup.de/2020/07/how-to-write-readable-code/

## Why do we write unreadable code?
I think we are mostly lazy, and so we use names from underlying structures like APIs or the databases, instead of using names that describe the current situation.

For example, the method “Cart->getUser(): User“: There is a “user_id“ in the underlying database table, so the developer chooses the name “getUser()“ where the User will be returned. The problem is “getX“ and “setX“ are terrible names for methods because they did not say you anything about the context of these methods. Maybe something like “Cart->orderUser(): OrderUser“ and at some point “Cart->orderApprovalUser(): OrderApprovalUser“ are better names. But keep in mind that this depends on your use cases. And please rename also e.g. your “user_id“ in the cart database table, so that you keep the code consistent and readable.

In Eric Evans’ book Domain-Driven Design he introduces the concept of a “Ubiquitous Language“ — a shared, common vocabulary that the entire team shares when discussing or writing software. This “entire team” is made up of designers, developers, the product owner and any other domain experts that exist at the organization. And take the note that it is important that your team have a domain expert! Mostly we are not the expert in the field we are writing software for, so we need some professional input. If you build an invoice-system you will need an expert in finance-questions and -laws.

## if ()

#### BAD:
```php
if ($userExists == 1) { }
```

#### BETTER: use "==="
```php
if ($userExists === 1) {}
```

#### BETTER: use the correct type
```php
if ($userExists === true) {}
```

#### BETTER: use something readable
```php
if ($this->userExists()) {}
```

#### BAD:
```php
$foo = 3;
if ($lall === 2) { $foo = 2; }
```

#### BETTER: use if-else
```php
if ($lall === 2) { 
    $foo = 2; 
} else {
    $foo = 3;
}
```

#### BETTER: only for simple if-else
```php
$foo = ($lall === 2) ? 2 : 3;
```

#### BETTER: use something readable
```php
$foo = $this->foo($lall);
```

#### BAD:
```php
if ($foo && $foo < 20 && $foo >= 10 || $foo === 100) {}
```

#### BETTER: use separate lines
```php
if (
    ($foo && $foo < 20 && $foo >= 10) 
    || 
    ($foo && $foo === 100)
) {}
```

#### BETTER: avoid duplications
```php
if (
    $foo
    &&
    (
        ($foo >= 10 && $foo < 20) 
        || 
        $foo === 100
    )
) {}
```

#### BETTER: use something readable
```php
if ($this->isFooCorrect($foo)) {}
```

## loop ()

#### BAD
```php
foreach ($cartArticles as $key => $value)
```

#### BETTER: use correct names
```php
foreach ($cartArticles as $cartIndex => $cartArticle)
```

#### BETTER: move the loop into a method
```php
cart->getArticles(): Generator<int, Article>
```

#### BAD
```php
foreach ($cartArticles as $cartArticle) {
    if ($cartArticle->notActive) {
        $this->removeArticle($cartArticle)
    }
}
```

#### BETTER: use continue
```php
foreach ($cartArticles as $cartArticle) {
    if (! $cartArticle->active) {
        continue;
    }

    $this->remove_article($cartArticle)
}
```

#### BETTER: use something readable
```php
cart->removeNonActiveArticles(): int;
```

## method ( )

#### BAD
```php
thisMethodNameIsTooLongOurEyesCanOnlyReadAboutFourCharsAtOnce(bool $pricePerCategory = false): float|float[]
```

#### BETTER: use shorter names if possible (long names mostly indicates that the method does more than one thing)
```php
cartNetPriceSum(bool $pricePerCategory = false): float|float[]
```

#### BETTER: use less parameter and use one return type
```php
cartNetPriceSum(): float
cartNetPriceSumPerCategory(): float[]
```

## class ()

#### BAD: 
```php
class \shop\InvoicePdfTemplate;

class \shop\InvoicePdfTemplateNew extends \shop\InvoicePdfTemplate;

class \shop\InvoicePdfTemplateSpecial extends \shop\InvoicePdfTemplateNew;
```

#### BETTER: use non-generic class names for non-generic classes
```php
class \shop\InvoicePdfTemplate;

class \shop\PdfTemplateCustomerX extends \shop\InvoicePdfTemplate;

class \shop\PdfTemplateActionEaster2020 extends \shop\PdfTemplateCustomerX;
```

#### BETTER: do not use multi-inheritance, because of side-effects
```php
class \shop\invoice\PdfTemplateGeneric

class \shop\invoice\PdfTemplate extends \shop\invoice\PdfTemplateGeneric;

class \shop\invoice\PdfTemplateCustomerX extends \shop\invoice\PdfTemplateGeneric;

class \shop\invoice\PdfTemplateActionEaster2020 extends \shop\invoice\PdfTemplateGeneric;
```

#### BETTER: use composition via interfaces
```php
interface \shop\invoice\PdfTemplateInterface;

class \shop\invoice\PdfTemplateGeneric implements \shop\invoice\PdfTemplateInterface;

class \shop\invoice\PdfTemplate extends \shop\invoice\PdfTemplateGeneric;

class \shop\invoice\PdfTemplateCustomerX extends \shop\invoice\PdfTemplateGeneric;

class \shop\invoice\PdfTemplateActionEaster2020 implements \shop\invoice\PdfTemplateInterface;
```

#### BETTER: use abstract or final
```php
interface \shop\invoice\PdfTemplateInterface;

abstract class \shop\invoice\PdfTemplateGeneric implements \shop\invoice\PdfTemplateInterface;

final class \shop\invoice\PdfTemplate extends \shop\invoice\PdfTemplateGeneric;

final class \shop\invoice\PdfTemplateCustomerX extends \shop\invoice\PdfTemplateGeneric;

final class \shop\invoice\PdfTemplateActionEaster2020 implements \shop\invoice\PdfTemplateInterface;
```

## Real-World-Example

https://github.com/woocommerce/woocommerce/blob/master/includes/class-wc-coupon.php#L505

#### BAD:
```php
/**
 * Set amount.
 *
 * @since 3.0.0
 * @param float $amount Amount.
 */
public function set_amount( $amount ) {
    $amount = wc_format_decimal( $amount );

    if ( ! is_numeric( $amount ) ) {
        $amount = 0;
    }

    if ( $amount < 0 ) {
        $this->error( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    if ( 'percent' === $this->get_discount_type() && $amount > 100 ) {
        $this->error( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    $this->set_prop( 'amount', $amount );
}
```

#### BETTER: fix phpdocs
```php
/**
 * @param float|string $amount Expects either a float or a string with a decimal separator only (no thousands).
 *
 * @since 3.0.0
 */
public function set_amount( $amount ) {
    $amount = wc_format_decimal( $amount );

    if ( ! is_numeric( $amount ) ) {
        $amount = 0;
    }

    if ( $amount < 0 ) {
        $this->error( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    if ( 'percent' === $this->get_discount_type() && $amount > 100 ) {
        $this->error( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    $this->set_prop( 'amount', $amount );
}
```

#### BETTER: use php types (WARNING: this is a breaking change because we do not allow string as input anymore)
```php
/**
 * @since 3.0.0
 */
public function set_amount( float $amount ) {
    if ( $amount < 0 ) {
        throw new WC_Data_Exception( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    if (
        $amount > 100
        &&
        'percent' === $this->get_discount_type()
    ) {
        throw new WC_Data_Exception( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    $this->set_prop( 'amount', $amount );
}
```

#### BETTER: merge the logic
```php
/**
 * @since 3.0.0
 */
public function set_amount( float $amount ) {
    if (
        $amount < 0
        ||
        (
            $amount > 100
            &&
            'percent' === $this->get_discount_type()
        )
    ) {
        throw new WC_Data_Exception( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    $this->set_prop( 'amount', $amount );
}
```

#### BETTER: make the logic readable
```php
/**
 * @since 3.0.0
 */
public function set_amount( float $amount ) {
    if (! is_valid_amount($amount)) {
        throw new WC_Data_Exception( 'coupon_invalid_amount', __( 'Invalid discount amount', 'woocommerce' ) );
    }

    $this->set_prop( 'amount', $amount );
}

private function is_valid_amount( float $amount ): bool {
    if ($amount < 0) {
        return false;
    }

    if (
        $amount > 100
        &&
        'percent' === $this->get_discount_type()
    ) {
        return false;
    }

    return true;
}
```

## Should we write code that everybody can read?

I don’t think that every non-programmer need to read the code, so we do not need to write the code this way. But we should remember that we could write our code, so that every non-programmer could read it.

## Summary

> There are only two hard things in Computer Science: cache invalidation and naming things.

― Phil Karlton

> “Indeed, the ratio of time spent reading versus writing is well over 10 to 1. We are constantly reading old code as part of the effort to write new code. …[Therefore,] making it easy to read makes it easier to write.”

― Robert C. Martin, Clean Code: A Handbook of Agile Software Craftsmanship