Clean code

Simple guide to improve our code

WHY?

Code is never permanent, it changes.

Easier improvements.

Welcome collaboration.

Programming is the art of telling another human what one wants the computer to do.
— Donald Knuth

WHY?

According to [Schach 1999] the maintenance part of the life-cycle accounts for 67% of the project cost.

WHat's clean code?

simple

it works

secure

commented

flexible

atomic

readable

no duplicated

no spaghetti code

follow standards

WHat's clean code?

simple

it works

secure

commented

flexible

atomic

readable

no duplicated

no spaghetti code

follow standards

clean code

No code is perfectly clean.

Code rots.

Code requires maintenance.

Requirements change and code gets obsolete.

clean code: BOY SCOUT RULE

Leave the code always cleaner than you found it.

Who owns the code?

Developer

~~Developer~~

Team

~~Team~~

Company / Client

clean code: DIMENSIONS

Brevity

Full feature

Performance

Robustness

Flexibility

Development time

clean code: STARTING

Brevity of code.

Start by writing the minimum functionality.
Extend as necessary in the other dimensions.

Keep It Simple, Stupid

— K.I.S.S principle

clean code: JUSTIFY

Do not be afraid of code reviews.

A developer should always be able to justify the code:

Never use "It works" as the only reason.
Never use "It's simple" as the only reason.

Ask for help or points of view if you feel your code smells.

Learning is the base of writing clean code.

STANDARDS

Agreed before the project.

Follow PSR-1/2 as guide

http://www.php-fig.org/psr/psr-1/
http://www.php-fig.org/psr/psr-2/

Follow drupal standards

https://www.drupal.org/coding-standards

STANDARDS

Make your life easier

Adjust IDE editor settings: tabs for spaces, how many, etc.
Use PHP CodeSniffer for checking: will detect wrong formatting (and more) and will notify you.
Strict code reviews.

array(
  'id' => 'identifier',
  'description' => 'short description',
  'text' => 'full text',
);

array(
  'id'          => 'identifier',
  'description' => 'short description',
  'text'        => 'full text',
);

MEANINGFUL NAMING

You can read your code like a book.

The name explicitly says what does/contains/purpose.

Length is not a problem.

Avoid type information in the names.

function get_book_title(array $books) {
  foreach ($books as $book) {
    if ($book->hasTitle()) {
      return $book->getTitle();
    }
  }
  return '';
);

function get_bt(array $bookList) {
  foreach ($bookList as $b) {
    if ($b->title()) {
      return $b->title;
    }
  }
  return '';
);

Say what you mean. Mean what you say.

COMMENTS

Write code as if you weren't going to use comments.

Code changes fast.

Comments barely do.

If it doesn't provide useful info, better don't put it.

Ex. of good comments: API, business weird logic,

function get_books_with_title(array $books) {
  return array_filter($books, function($book) {
    return $book->hasTitle();
  });
);

function get_bt(array $bookList) {
  // Initialise aux list.
  $list = array();
  // Loop the list of books.
  foreach ($bookList as $b) {
    // Check if it has title.
    if ($b->title()) {
      $list[] = $b->title;
    }
  }
  return $list;
);

functions/methods

function get_book_list($data) {
  if ($data['list']) {
    return (array) $data['list'];
  }
  return array();
);

function get_book($id) {
  return ($id) ? Book::getInstance($id) 
    : Book::getEmptyInstance();
}

function get_book_list($data) {
  if ($data['list']) {
    return (array) $data['list'];
  }
  return FALSE;
);

function get_book($id) {
  return ($id) ? Book::getInstance($id) 
    : NULL;
}

Always return the same data type.

foreach (get_book_list($data) as $book) {
  print get_book($book['id'])->getTitle();
}

functions/methods

function redirect($path, $http_code) {
  if ($path) {
    header('Location: ' . $path, TRUE, 
      $http_code);
  }
}

function redirect_on_login() {
  redirect('/login', 302);
);

function redirect($path = '/login', 
  $http_code = 302) {
  header('Location: ' . $path, TRUE, 
    $http_code);
}

function redirect_on_login() {
  redirect();
);

Avoid default values. They hide functionality.

functions/methods

function get_book_info($data) {
  if ($data['id']) {
    return load_book_info($data);
  }
  if ($data['info']) {
    return $data['info'];
  }
  return array();
}

function get_book_info($data) {
  if (!$data['id']) {
    return array();
  }
  else if ($data['info']) {
    return $data['info'];
  }
  else {
    return load_book_info($data);
  }  
}

Place default value at the end for visibility.

functions/methods

function setcookie(Cookie $cookie) {
  ...
);

function get_distance(Coordinate $origin,
  Coordinate $destination) {
    ...
}

function setcookie($name, $value, 
  $expire, $path, $domain, $secure, 
  $httponly) {
    ...
);

function get_distance($originX, $originY,
  $destinationX, $destinationY) {
    ...
}

One argument (monad)

Two arguments (dyad)

Three arguments (triad)

- Think carefully. Can we use objects or lists?

Four arguments

- Probably you can split your function or join arguments.

OOP vs PROCEDURAL

Procedural

OOP

Stateless

Stateful

class Number {

  public function __construct(int $number) {
    $this->number = $number;
  }

  public function double() {
    return 2 * $this->number;
  }
}

$number = new Number(2)
echo $number->double(); // prints 4
$number = new Number(3);
echo $number->double(); // prints 6

function double(int $number) {
  return 2 * $number;
);

echo double(2); //prints 4
echo double(3); //prints 6

OOP vs PROCEDURAL

Procedural

OOP

One input = One output

One input >= One output

class Number {

  private $i;

  public function increment(int $increment) {
    $this->i += $increment;
    return $this->i;
  }
}

$number = new Number();
echo $number->increment(2); // prints 2
echo $number->increment(2); // prints 4

function incremental(int $increment) {
  static $i = 0;
  $i += $increment; 
  return $increment;
);

echo increment(2); //prints 2
echo increment(2); //prints 4

OOP vs PROCEDURAL

Procedural

OOP

Please don't use globals

- breaks encapsulation.

- naming collisions.

- anyone can change them.

Singleton

class Number implements Singleton {

  private $i;

  public function increment(int $increment) {
    $this->i += $increment;
    return $this->i;
  }
}

$number = Number::getInstance();
echo $number->increment(2); // prints 2
$number = Number::getInstance();
echo $number->increment(2); // prints 4

function incremental(int $increment) {
  global $i;
  $i += $increment; 
  return $increment;
);

echo increment(2); //prints 2
echo increment(2); //prints 4

OOP

static methods are procedural functions.

too many static methods is a smell indicator.

class Number {

  static function abs(int $number) {
    return ($number * $number) / $number;
  }

  static function decimals(float $number) {
    return $number % self::int($number);
  }

  static function int(float $number) {
    return (int) $number;
  }
}

class Number {

  protected $value;

  public function abs() {
    return abs($this->value);
  }

  static function getTypes() {
    return ['float', 'int', 'real'];
  }
}

OOP

do not extend constants

class Car  {

  protected $wheels = 4;

  public function getWheels() {
    return $this->wheels;
  }
}

class Motorbike extends Car {

  protected $wheels = 2;
}

$car = new Car();
echo $car->getWheels(); // prints 4
$motorbike = new Motorbike();
echo $motorbike->getWheels(); // prints 2

class Car {

  const WHEELS = 4;

  public function getWheels(int $increment) {
    return static::WHEELS;
  }
}

class Motorbike extends Car {

  const WHEELS = 2;
}

$car = new Car();
echo $car->getWheels(); // prints 4
$motorbike = new Motorbike();
echo $motorbike->getWheels(); // prints 2

S.O.L.I.D

Single responsibility

Open-closed

Liskov substitution

Interface segregation

Dependency inversion

Single responsibility

Every class is responsible for one part of the functionality.

class Dashboard {

  public function getItems() { ... }

}

class Version {

  public function getMinorVersion() { ... }

  public function getMajorVersion() { ... }

  public function getBuildVersion() { ... }

}

class Dashboard {

  public function getItems() { ... }

  public function getBuildVersion() { ... }
}

Open/closed principle

entities should be open for extension, but closed for modification

interface Product {

  public function getPrice();
}

class Cart {

  function getTax(Product[] $products) {
    $tax = 0;
    foreach ($products as $product) {
      if (is_a($product, 'Book')) {
        $tax += $product->getPrice() * 0.21;
      }
      else if (is_a($product, 'Food')) {
        $tax += $product->getPrice() * 0.07;
      }
    }
    return $tax;
  }
}

interface Product {

  public function getPrice();

  public function getTax();
}

class Cart {

 function getTax(Product[] $products) {
    $tax = 0;
    foreach ($products as $product) {
      $tax += $product->getTax();
    }
    return $tax;
  }
}

INTERFACE SEGREGATION

no entity should be forced to depend on methods it does not use

interface Task {

  public function preprocess();

  public function process();

  public function print();

  public function fax();

  public function pdf();
}

interface Task {
  public function preprocess();

  public function process();
}

interface Print extends Task {
  public function print();
}

interface Fax extends Task {
  public function fax();
}

interface PDF extends Task {
  public function pdf();
}

DEPENDENCY INVERSION

Depend upon Abstractions. Do not depend upon concretions

interface Car { ... }

class Volkswagen implements Car { ... }

class Garage {

  public function fix(Volkswagen $car) {
    ...
  }

  public function fixVolkswagen($car) {
    ...
  }
}

interface Car { ... }

class Volkswagen implements Car { ... }

class Garage {

  public function fix(Car $car) {
    ...
  }
}

LISKOV SUBSTITUTION

objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program

abstract Car {

  public function getModel() : string {
    return $this->model; 
  }
}

class SEAT extends Car {

  public function getModel($type) {
    if (!$this->model) {
      throw new Exception('No model set');
    }
    return array($this->model);
  }
}

abstract Car { 

  public function getModel() : string {
    return $this->model; 
  }
}

class SEAT extends Car {

  public function getModel() : string {
    return random($this->spainCities);
  }
}

Sources

http://www.slideshare.net/hebel/clean-code-vortrag032009pdf

http://blog.codinghorror.com/the-best-code-is-no-code-at-all/

https://en.wikipedia.org/wiki/SOLID_(object-oriented_design)

https://cleancoders.com/category/clean-code

https://scotch.io/bar-talk/s-o-l-i-d-the-first-five-principles-of-object-oriented-design

http://www.irahul.com/2005/10/software-development-vs-software.html

Questions

Gorka Guridi

Drupal Architect, Cameron & Wilding Ltd.

www.cameronandwilding.com

@cameronwilding

Clean Code

By Gorka Guridi

Clean Code

Why we should, how do we do it and examples about how to write clean code.

Gorka Guridi

github.com/gguridi

Clean code

WHY?

WHY?

WHat's clean code?

WHat's clean code?

clean code

clean code: BOY SCOUT RULE

clean code: DIMENSIONS

clean code: STARTING

clean code: JUSTIFY

STANDARDS

STANDARDS

MEANINGFUL NAMING

COMMENTS

functions/methods

functions/methods

functions/methods

functions/methods

OOP vs PROCEDURAL

Procedural

OOP

OOP vs PROCEDURAL

Procedural

OOP

OOP vs PROCEDURAL

Procedural

OOP

OOP

OOP

S.O.L.I.D

Single responsibility

Open/closed principle

INTERFACE SEGREGATION

DEPENDENCY INVERSION

LISKOV SUBSTITUTION

Sources

Questions

Clean Code

More from Gorka Guridi