DEV Community

Lars Moelleken
Lars Moelleken

Posted on

How to write readable code?

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

if ($userExists == 1) { }
Enter fullscreen mode Exit fullscreen mode

BETTER: use "===" 

if ($userExists === 1) {}
Enter fullscreen mode Exit fullscreen mode

BETTER: use the correct type 

if ($userExists === true) {}
Enter fullscreen mode Exit fullscreen mode

BETTER: use something readable 

if ($this->userExists()) {}
Enter fullscreen mode Exit fullscreen mode

BAD: 

$foo = 3;
if ($lall === 2) { $foo = 2; }
Enter fullscreen mode Exit fullscreen mode

BETTER: use if-else 

if ($lall === 2) { 
    $foo = 2; 
} else {
    $foo = 3;
}
Enter fullscreen mode Exit fullscreen mode

BETTER: only for simple if-else 

$foo = ($lall === 2) ? 2 : 3;
Enter fullscreen mode Exit fullscreen mode

BETTER: use something readable 

$foo = $this->foo($lall);
Enter fullscreen mode Exit fullscreen mode

BAD: 

if ($foo && $foo < 20 && $foo >= 10 || $foo === 100) {}
Enter fullscreen mode Exit fullscreen mode

BETTER: use separate lines 

if (
    ($foo && $foo < 20 && $foo >= 10) 
    || 
    ($foo && $foo === 100)
) {}
Enter fullscreen mode Exit fullscreen mode

BETTER: avoid duplications 

if (
    $foo
    &&
    (
        ($foo >= 10 && $foo < 20) 
        || 
        $foo === 100
    )
) {}
Enter fullscreen mode Exit fullscreen mode

BETTER: use something readable 

if ($this->isFooCorrect($foo)) {}
Enter fullscreen mode Exit fullscreen mode

loop ()

BAD 

foreach ($cartArticles as $key => $value)
Enter fullscreen mode Exit fullscreen mode

BETTER: use correct names 

foreach ($cartArticles as $cartIndex => $cartArticle)
Enter fullscreen mode Exit fullscreen mode

BETTER: move the loop into a method 

cart->getArticles(): Generator<int, Article>
Enter fullscreen mode Exit fullscreen mode

BAD 

foreach ($cartArticles as $cartArticle) {
    if ($cartArticle->notActive) {
        $this->removeArticle($cartArticle)
    }
}
Enter fullscreen mode Exit fullscreen mode

BETTER: use continue 

foreach ($cartArticles as $cartArticle) {
    if (! $cartArticle->active) {
        continue;
    }

    $this->remove_article($cartArticle)
}
Enter fullscreen mode Exit fullscreen mode

BETTER: use something readable 

cart->removeNonActiveArticles(): int;
Enter fullscreen mode Exit fullscreen mode

method ( )

BAD 

thisMethodNameIsTooLongOurEyesCanOnlyReadAboutFourCharsAtOnce(bool $pricePerCategory = false): float|float[]
Enter fullscreen mode Exit fullscreen mode

BETTER: use shorter names if possible (long names mostly indicates that the method does more than one thing) 

cartNetPriceSum(bool $pricePerCategory = false): float|float[]
Enter fullscreen mode Exit fullscreen mode

BETTER: use less parameter and use one return type 

cartNetPriceSum(): float
cartNetPriceSumPerCategory(): float[]
Enter fullscreen mode Exit fullscreen mode

class ()

BAD: 

class \shop\InvoicePdfTemplate;

class \shop\InvoicePdfTemplateNew extends \shop\InvoicePdfTemplate;

class \shop\InvoicePdfTemplateSpecial extends \shop\InvoicePdfTemplateNew;
Enter fullscreen mode Exit fullscreen mode

BETTER: use non-generic class names for non-generic classes 

class \shop\InvoicePdfTemplate;

class \shop\PdfTemplateCustomerX extends \shop\InvoicePdfTemplate;

class \shop\PdfTemplateActionEaster2020 extends \shop\PdfTemplateCustomerX;
Enter fullscreen mode Exit fullscreen mode

BETTER: do not use multi-inheritance, because of side-effects 

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;
Enter fullscreen mode Exit fullscreen mode

BETTER: use composition via interfaces 

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;
Enter fullscreen mode Exit fullscreen mode

BETTER: use abstract or final 

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;
Enter fullscreen mode Exit fullscreen mode

Real-World-Example

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

BAD: 

/**
 * 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 );
}
Enter fullscreen mode Exit fullscreen mode

BETTER: fix phpdocs 

/**
 * @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 );
}
Enter fullscreen mode Exit fullscreen mode

BETTER: use php types (WARNING: this is a breaking change because we do not allow string as input anymore) 

/**
 * @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 );
}
Enter fullscreen mode Exit fullscreen mode

BETTER: merge the logic 

/**
 * @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 );
}
Enter fullscreen mode Exit fullscreen mode

BETTER: make the logic readable 

/**
 * @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;
}
Enter fullscreen mode Exit fullscreen mode

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

Top comments (2)

Collapse
 
xanderyzwich profile image
Corey McCarty •

There's a weird thing in readability that you learn to quickly read the form that you use/read most. This means that if you have multiple repositories full of another format then your "ideal" format will be more difficult. Standardize your code format for the team.

If ternaries aren't used elsewhere then it can take longer to remember how to read it than just having the if/else.

Things like right side open VS left side open can be acceptable either way if you are accustomed to it.

Collapse
 
suckup_de profile image
Lars Moelleken •

Be consistent! Yes, but this cannot be an argument for bad code style.

e.g.:

bad:

.example_1,.example_2{background: url(images/example.png) center center no-repeat, url(images/example-2.png) top left repeat, url(images/example-3.png) top right no-repeat;}

not that bad:

.example_1, 
.example_2 {     
 background: url(images/example.png) center center no-repeat,                            
             url(images/example-2.png) top left repeat,                            
             url(images/example-3.png) top right no-repeat;
}

-> dev.to/suckup_de/do-not-fear-the-w...

Read next

aaronreddix profile image

How to Build Progressive Web Apps in 2024? A Step-byStep Guide

Aaron Reddix -

nhannguyendevjs profile image

JavaScript Design Patterns - Structural - Decorator

Nhan Nguyen -

milasuperstar profile image

Top Open Source Prompt Engineering Guides & Tools🔧🏗️🚀

Mila Wu -

stephrp12 profile image

What Will You Choose?

StephRP12 -