Clean code

Prato NV
Wouter Groeneveld & Luk Weyens
22/09/2017

Introductie

GSM Uit aub

Verstand Aan aub
Vragen stellen toegelaten!

Inhoudsopgave

  1. Clean code: definities
  2. Clean code: principes
  3. Unit testing: definities
  4. Unit testing: voorbeelden

Clean code deel 1:
definitie

Wat is clean code?
Clean code is code that is easy to understand and easy to change.
Wat is niet clean code?

			function process($obj, $continue = FALSE) {
				// echo "starting the process";
				if($obj->generate() == NULL) {
					return -1;
				}
				$result = $obj->generate();
				if($continue) {
					return $result->generate();
				}
				return $result;
			}
						
Wat is wel clean code?

	function getBookByISBN($isbn) {
		$book = array_search($isbn, $this->books);
		if($book == FALSE) {
			throw new BookNotFoundException("book was not found!");
		}
		return $book;
	}
						

Leesbaarheid!

Code gets read a lot (at least whenever someone is writing more code), so any school of clean code should emphasize readability. Cleaning up a little wherever you go is required to keep code clean.

Meetbaarheid:
"aantal WTFs per minuut"

Clean code deel 2:
principes

1. Naamgeving:
wat is de ideale naamgeving?

  • Het moet correct weerspiegelen waar het voor dient
  • Het leunt aan bij de taal die gesproken wordt in het domein

1. Naamgeving: methods/variabelen


$result = $processor->process();
					
VS

$order = $library->checkOut();
					

1. Naamgeving: comments

1. Naamgeving: comments


	/**
	 *
	 * Prolong the book
	 *
	 * @param    Book  		$i The book to prolong
	 * @param    DateTime  		$j The date to prolong to
	 * @param    Person		$k The person to prolong the book to
	 * @return   nothing if OK
	 *
	 */
	function prolong($i, $j, $k) {
		//Check if this book has been checked out by the same person
		if($i->person != $k)
			//BUSTED!
			return "This is not a book you checked out."
		else
			//Everything OK, prolong the book
			$book->checkOutDate = $date;
	}
						

1. Naamgeving: comments


	function prolong($book, $date, $person) {
		if(!$book->isCheckedOutBy($person))
			return "This is not a book you checked out."
		else
			$book->checkOutDate = $date;
	}
						

2. Functies:
wat is een ideale functie?

2. Functies: verantwoordelijkheden


class Book {
	function prolong($date, $person) {
		// does only one thing: prolonging the book
	}
}
					

2. Functies: argumenten


function checkoutBook($title, $isbn, $person, $enoughCredit = FALSE) {
	// ...
}
					
VS

class Person {
	function checkoutBook($book) {
		// ...
	}
}
					

2. Functies: Side effects

POGING 1

	class Book {
			function prolong($date, $person) {
				//prolong implementatie
				if($this->isOverDue()){
					$person->credit -= 10;
				}
		}
	}
						
POGING 2

	class Book {
		function prolong($date, $person) {
			//prolong implementatie
		}
	}

	if($book.isOverdue($date)){
		$person->applyFine();
	}
	$book->prolong($date, $person);
						

oefeningen deel 1

Oefeningen deel 1: story 1

  • Gegeven: Empire, Sith, Jedi classes vol funcs.
  • Uw taak: verduidelijken! Wat doet wat? Naamgeving!
  • Unit testen groen houden! (cmd: > phpunit tests)

PHPUNIT - recap

live demo
http://phpunit.de/manual/

Oefeningen deel 1: story 2

  • maak een function op Empire, 'convert($jedi)'
       Wat zou dit volgens u moeten doen?
  • maak een function op Jedi, 'equip($saber)'.
       Wat zou dit volgens u moeten doen?
  • Vergeet niet de testen uit te breiden!

Oefeningen deel 1 - discussie

  • Zijn alle testen nog groen? Heb je er bijgeschreven?
  • Naamgeving - ook doorgevoerd in testen?
  • Onthou principes voor volgende oefeningen!

3. Scoping - poging 1



	public class BookRepository {
		public $db;
		public $bookCache = [];
	}

	$bookRepositoryInstance->db->query('where isOverdue = TRUE');
						

3. Scoping - poging 2


public class BookRepository {
	private $db;
	public function getOverdueBooks() {
		return $db->query('where isOverdue = TRUE');
	}
}

$bookRepositoryInstance->getOverdueBooks();
						

3. Scoping - global #fail


$checkoutDate = NULL;
function isOverdue($book) {
	global $checkoutDate;
	// some domain logic here
	$checkoutDate = new Date();
}

$book->setCheckoutDate($checkoutDate); 
					

Zie ook static e.a.

4. OO design - poging 1


	$bookid = $_POST['bookid'];
	function prolong($book, $date, $person) {
			$book->checkOutDate = $date;
			$person->bookList->push($book);
		}
	}
	prolong($db->getby($bookid), $_POST['date'], $person);

	echo "thank you it has been prolonged"

						

4. OO design - poging 2


	class Book {
		function prolong($date, $person) {
			$person->addToBookList($this);
		}
	}
	$book = new Book();
	$book->prolong($_POST['date'], $person);
	$view->renderThankYouPage();
						

	class BookTest extends TestCase {
		public void testProlongAddsToPersonBookList() {
			// ...
		}
	}
						

4. OO design: polymorphism - poging 1


	function getAisle($book) {
		switch($book->type) {
			case 'nonficition': return 400;
			case 'fiction': return 234;
		}
	}

	getAisle({ type: 'nonfiction' });
						

4. OO design: polymorphism - poging 2


	abstract class Book {
		abstract public function getAisle();
	}
	class FictionBook extends Book {
		function getAisle() {
			return 234;
		}
	}
	class NonFictionBook extends Book {
		function getAisle() {
			return 400;
		}	
	}

	new NonFictionBook()->getAisle();
						

4. OO design; Trainwrecks


$repository->getBookById(124)->getAuthor()->getAllBooks()->filterByName('train');
					

4. OO design; Error handling - poging 1


	function getBookById($id) {
		if($id == NULL) return -1;

		return $db->getById($id);
	}
						

4. OO design; Error handling - poging 2


function getBookById($id) {
	if($id == NULL) throw new InvalidIdException("id cannot be null!");

	return $db->getById($id);
}
						

Refactoring

Structuur veranderen, zonder inhoud te wijzigen!

Oefeningen deel 2

Dierentuin - story 1

  • maak een Dierentuin class
  • kan verschillende Dieren (class) ontvangen (function)
  • elk dier heeft een grootte en naam:
    Neushoorn (40), Giraf(25), Poema (10)
  • elke dierentuin heeft x beschikbareRuimte afhankelijk van grootte van het dier
    wat doe je bij het ontvangen van een te groot dier?

class Dierentuin {
	public function bezoek() {
		// return [] of dieren
	}
	public function ontvangDier($dier) {
		// ??
	}
}
						

Dierentuin - story 2

  • voeder functie op dierentuin
    return TRUE of FALSE indien genoeg voedsel voor elk ontvangen dier in de tuin
  • voedsel heeft een voedingswaarde
    Elk dier eet even veel waarde als zijn grootte
  • Verzin voedsel implementaties om alle edge cases te kunnen testen!

class Dierentuin {
	public function voeder($voedsel) {
		return TRUE;	// ??
	}
}
						

Dierentuin - story 3

  • Dieren zijn carnivoeren, herbivoren of omnivoren!
  • voedsel is ofwel vlees- of plant-gebaseerd
  • Wat doe je als je een carnivoor sla geeft, of een herbivoor een stukje kip?
  • Focus op de testen! Alle gevallen onder test brengen.

	public function voeder($voedsel) {
		// wat nu??
	}
						

Dierentuin - story 4

  • Dieren zijn allergisch aan bepaald voedsel!
  • Wat doe je als een dier voedsel eet wat het niet verdraagt?
    Hoe ga je om in de Dierentuin klasse met een dier minder?
  • Wat doe je in de voeder implementatie?

class Dier {
	public function isAllergischAan($voedsel) {
		return TRUE;	// ??
	}
}
						

Dierentuin - discussie

  • Polymorfisme niet vergeten?
  • Genoeg testen geschreven voor elk geval?
  • Exceptions gebruikt ipv return code?

Het pareto principe

80% van het werk, met
20% van de inspanning
You cannot write perfect software. Therefore, do not waste energy trying;
be pragmatic.

DRY: Don't Repeat Yourself

"Every piece of knowledge must have a single, unambiguous, authoritative representation within a system."
---- DAG 2 ----

Clean code deel 3:
Unit testing

Waarom unit testen?

Waarom unit testen?

  • feedback!
  • Denk in code vanuit API standpunt
  • Makkelijk voor pair om te volgen
  • Alle mogelijke paden gedekt

Eigenschappen van een goede test

  • Ontdek sneller bugs
  • Leesbaar
  • Geautomatiseerd
  • Snel & gefocuset
  • Herhaalbaar
  • Volgorde onafhankelijk
  • Productie code makkelijker wijzigbaar

Eigenschappen van een goede test
Geautomatiseerd

Eigenschappen van een goede test
"living documentation"

Test Driven Development (TDD)

TDD: "bugfix" edge case

live demo

Soorten testen

 

E2E
Integration
Unit

unit testing

  • Onafhankelijk van externen (db, webservice, ...)
  • Snel!
  • Véél testen
  • Test normaal pad & limieten
  • "actieve vijand van de code"

integration testing

  • Test geïntegreerd met externen (db, webservice, ...)
  • Test integratie twee verschillende lagen
  • Trager dan unit tests
  • Minder test cases

end to end testing

  • Test hele applicatie!
  • niet alle limieten
  • traag, moelijker onderhoudbaar
  • Test integratie alle lagen

OEFENING:
SPREADSHEET

Story 1: simple spreadsheet

  • Onbeperkt aantal rijen/kolommen
  • Rij = nummer 1 -> 9999...
  • Kolom = Cijfer A -> Z, AA -> ZZ, ...
  • Cellen zijn standaard leeg ("")
  • Cellen kunnen tekst opslaan
  • Mogelijk om inhoud van cel uit te lezen

Story 2: numerics and literal values

  • Numerische cellen worden automatisch herkend:
    Tekst " 124 " (met spaties) wordt het getal 124 (zonder spaties)
  • Mogelijk om literaire inhoud van cel uit te lezen
    Cel met tekst " 124 " heeft literaire waarde " 124 " en waarde 124

Story 3: Formulas

  • "=234" is een formule, " =234" (spatie) is tekst
  • Waarde van formule "=234" is het getal 234. Literaire waarde = de formule
  • Soorten:
    1. Constanten =124
    2. Met haakjes =((124))
    3. Simpele berekeningen =3+5, =5-3, =4*2
    4. Complexe berekeningen =7*(2+3)*((((2+1))))

Story 3: Formulas - berekeningen


	public function testFormula_evaluatesSimpleCalculation() {
		$formula = "3+5";

		$this->assertEquals(8, eval('return ' . $formula . ';'));
	}

						
Gebruik nooit eval() in productiecode!
Waarom niet?

Story 4: Formula errors

Wanneer een formule een fout bevat, moet de waarde van de cel "#ERROR" zijn.

bvb. =((124)-()) of =7* of delen door 0.

Story 5: dependencies

=A2

  • Waarde = waarde van cel waar je naar verwijst.
  • Indien waarde A2 verandert, verandert deze cel ook.
  • Referenties naar referenties = mogelijk! A1 -> A2 -> A3
  • Circulaire referneties vermijden.
    Geef "#CIRCULAR" fout als waarde.

Story 6: Functions

  • Sum(range)
    Bijvoorbeeld =SUM(A1:A6)
  • Average(range)
    Bijvoorbeeld =AVG(A1:D1)

Spreadsheet - discussie

  • Genoeg testen geschreven voor elk geval?
  • Exceptions gebruikt ?
  • Is je code leesbaar voor je buurman?
  • Was dit makkelijker met focus op schrijven van testen of niet?

DAG 3

time spent by programmers at work

Clean code herhaling (JS):
principes

Programmers are really authors, and your target audience is not the computer it is other programmers (and yourself).
 

1. Naamgeving: methods/variabelen


	function render() {
		renderRest();
	}
	function renderRest() {
		document.getElementById('#ak').innerHTML = 'Hello PXL!';
	}
					
VS

	function renderHelloPXLPage() {
		document.getElementById('#content').innerHTML = 'Hello PXL!';
	}
					

2. Functies: verantwoordelijkheden


	function getBodyElements() {
		return document.querySelector('.body').forEach(function(i) {
			bodies++;	// ???
		});
	}
					
VS

	function getBodyElements() {
		return documents.querySelector('.body');
	}
	function getAmountOfBodyElements() {
		return getBodyElements().length;
	}
					

2. Functies: argumenten


	function prolongBook() {
		if(arguments[1]) {
			var overdue = true;
		}
		if(arguments[0]) {
			arguments[0].prolong(overdue);
		}
	}
					
VS

	function prolongBook(book, isOverdue) {
		// ...
	}
					

2. Functies: Side effects


	function renderContentCallback() {
		if(!this.rendered) {
			reRenderContent();
		}
		this.rendered = true;
	}
					
Zelfde principe in elke taal...

3. Scoping - poging 1


	var obj = {
		isRendered: false,

		render: function() {
			if(!this.isRendered) {
				refresh();
			}
			this.isRendered = true;
		}
	};
						
Is isRendered voldoende afgeschermd?

3. Scoping - poging 1

Het antwoord is Neen:

  1. delete obj.isRendered en *POOF*
  2. obj.isRendered = true en *POOF*

3. Scoping - poging 2


	obj = (function() {
		var isRendered = false;
		var render = function() {
			// ...
		}

		return {
			render: render
		};
	})();
						

3. Scoping - poging 2


	(function() {
		// niemand kan aan mij! FUNCTION LEVEL SCOPE
	})();
						
Expose enkel wat jij wil als publieke interface:

	myAPI = (function() {
		return {
			// enkel DIT is aanspreekbaar
		};
	})();
						

3. Scoping

Pas op met PHP!

	function main() {
	    if (TRUE) {
	        $i = 3;
	    }
	    echo $i;
	}
	main();			// wat doet dit?
						
Waarom?

3. Scoping

Pas op met Javascript!

	var main = function () {
	    if (true) {
	        var i = 3;
	    }
	    console.log(i);
	}
	main();			// wat doet dit?
    				
wordt door interpreter dit:

	var main = function () {
		var i = undefined;
	    if (true) {
	        i = 3;
	    }
	    console.log(i);
	}
    				

3. Scoping

VS een taal als C:

	void main() {
	    if (true) {
	        int i = 3;
	    }
	    printf("%d", i);	// wat doet dit?
	}
						
Waarom? Statisch getypeerd!

3. Scoping - global #fail


	function calculateBookPrice() {
		discount = 10;	// oops??
		return this.basePrice + calculateOverduePrice() - discount;
	}
					
Wat is de waarde van window.discount?

3. Scoping - Mix/machting tags



					
VS

Pear PHP standaarden:
http://pear.php.net/manual/en/standards.php

4. OO design - poging 1



	var bookid = document.querySelector('#bookid').innerHTML;
	var prolong = function(book, date, person) {
			book.checkOutDate = date;
			person.bookList.push(book);
		}
	};
	doAjaxCall('getBookById.json', bookid, function(book) {
		prolong(book, someDate, person);
	});
	document.querySelector('#messages').innerHTML = "thank you";

						
Wat gaat er mis?

4. OO design - poging 2


	// ------ domain/book.js ------ 
	var Book = {
		prolong: function(date, person) {
			person.addToBookList(this);
		}
	};
	// ------ controller/bookid.js ------
	var bookid = view.getBookId();
	doAjaxCall('getBookById.json', bookid, function(book) {
		book.prolong(someDate, person);
	});
	view.renderThankYouPage();
						
Wat gaat er mis?

4. OO design - poging 3


	doAjaxCall('getBookById.json', bookid, function(book) {
		book.prolong(someDate, person);
		view.renderThankYouPage();
	});
						

4. OO design - Testen!


	describe("Book tests", function() {
	  it("should add book to person list", function() {
	  	var book = Object.create(Book);
	  	book.prolong(someDate, person);

	    expect(person.getBookList()).toContain(book);
	  });
	});						
						

4. OO design

Héél belangrijk in goed onderhoudbare JS code:

  1. Scheiding domein/callbacks - async stuff
  2. Scheiding domein/front - DOM manipulatie
  3. Scheiding domein/frameworks - niet (makkelijk) testbaar!

Moeilijk testbare code => beter eerst testen geschreven??

Unit testing in JS

jasmine logo


		describe("A suite", function() {
		  it("contains spec with an expectation", function() {
		    expect(true).toBe(true);
		  });
		});
						
Beschrijft gedrag

Test Driven Development (TDD)
remember?

TDD: "bugfix" edge case

live demo - remember "Period"?

Oefening - Dierentuin HERWERKEN IN JS

Dierentuin - story 1 - remember?

  • maak een Dierentuin class (ES6!)
  • kan verschillende Dieren ontvangen
  • elk dier heeft een grootte en naam:
    Neushoorn (40), Giraf(25), Poema (10)
  • elke dierentuin heeft x beschikbareRuimte afhankelijk van grootte van het dier
    wat doe je bij het ontvangen van een te groot dier?

class Dierentuin {
	constructor() {}

	bezoek() {
		// return [] of dieren
	}
	ontvangDier(dier) {
		// ??
	}
}
						

Dierentuin - story 2

  • voeder functie op dierentuin
    return true of false indien genoeg voedsel voor elk ontvangen dier in de tuin
  • voedsel heeft een voedingswaarde
    Elk dier eet even veel waarde als zijn grootte
  • Verzin voedsel implementaties om alle edge cases te kunnen testen!

class Dierentuin {
	voeder(voedsel) {
		return true;	// ??
	}
}
						

Dierentuin - story 3

  • Dieren zijn carnivoeren, herbivoren of omnivoren!
  • voedsel is ofwel vlees- of plant-gebaseerd
  • Wat doe je als je een carnivoor sla geeft, of een herbivoor een stukje kip?
  • Focus op de testen! Alle gevallen onder test brengen.

	public function voeder($voedsel) {
		// wat nu??
	}
						

Dierentuin - story 4

  • Dieren zijn allergisch aan bepaald voedsel!
  • Wat doe je als een dier voedsel eet wat het niet verdraagt?
    Hoe ga je om in de Dierentuin klasse met een dier minder?
  • Wat doe je in de voeder implementatie?

class Dier {
	public function isAllergischAan($voedsel) {
		return TRUE;	// ??
	}
}
						

Dierentuin - ES6 crash course

Van PHP naar JS met ES6

JS VS PHP:

class Dier {
	constructor(arg) { }		// __construct()
	eet(voedsel) {			// public function eet($voedsel)
		this.boer();		// $this->boer();
	}
	boer() {			// public function boer()
		console.log('burp'); 	// echo "burp";
	}
}
new Dier().eet();			// new Dier()->eet();
						
Herwerk je code!
http://es6-features.org/#ClassDefinition.

Dierentuin - discussie

  • Genoeg testen geschreven voor elk geval?
  • Was dit makkelijk van PHP naar JS over te zetten?
  • Gedacht aan classes en inheritance?

JS Resources

Eerst leren kruipen, dan lopen!

 

Linting tools: JSLint

If a feature is sometimes useful and sometimes dangerous and if there is a better option then always use the better option.
Code Quality tools demo @ JSLint voor:

		"use strict";
		function bla() {

		  mwaha = "hi";
		  if(mwaha == "hi") {
		    console.log("hi there too")
		  }
		}
					

Soorten testen - remember?

 

E2E
Integration
Unit

integration testing - remember?

  • Test geïntegreerd met externen (db, webservice, ...)
  • Test integratie twee verschillende lagen
  • Trager dan unit tests
  • Minder test cases

unit testing - hoe één stukje testen?


	var Book = {
		prolong: function(person) {
			var date = Date.now();
			this.dbHandle.prolong(date, person.id);
		}
	};
						
Ik heb dbHandle.prolong() apart getest, wat nu?

unit testing - hoe één stukje testen?

Injecteer een andere dbHandle: een "mock".


 	var book = Object.create(Book);
 	book.dbHandle = {
 		prolong: function(date, personId) {
 			// Captured arguments! Assert stuff here
 		}
 	};
						

unit testing - met Jasmine

Laat Jasmine "spies" aanmaken
https://jasmine.github.io/api/2.6/global.html#spyOn


it("should call prolong from repository", () => {
	let personId;
	const dbHandle = spyOn('dbHandle', 'prolong').and.callFake((theDate, thePersonId) => {
		personId = thePersonId;
	});
	book.dbHandle = dbHandle;
	book.prolong(person);

	expect(personId).toBe(person.id);
});
						

unit testing - met PHPUnit

https://phpunit.de/manual/current/en/test-doubles.html


    public function testStub()
    {
        $mock = $this->createMock(DbHandle::class);
        $mock->method('prolong')
        		->with($this->anything())
        		->with($this->person.id);
    	$book = new Book($mock);

        $this->assertEquals('foo', $book->prolong());
    }
						

unit testing - in Symfony

Welke stukken kan ik op die manier testen...
in een Symfony project?

symfony logo

unit testing - in Symfony

  • De entityManager ObjectManager klasse
  • Stukjes domein logica uit controllers halen die elders getest zijn
  • ...

      $bookRepositoryMock = $this->getMockBuilder(EntityRepository::class)
            ->disableOriginalConstructor()
            ->getMock();
      $bookRepositoryMock->expects($this->once())
            ->method('find')
            ->will($this->returnValue($book));

     $bookController = new BookController($bookRepositoryMock);
						
http://symfony.com/doc/current/testing/database.html

Oefening - CODE REVIEWS

Code reviews

  • switch computers!
  • Zoek actief naar stukken die goed en minder zijn.
    Denk aan clean code & unit testen!
  • Nog geen code wijzigen!
  • Neem de tijd om alles onder de loep te nemen, incl testen.
  • Noteer in een open NOTEPAD scherm: + WAAROM?

Code reviews - discussie

  • Wie heeft de unit testen bekeken?
    Ontbreken er een aantal?
  • Wie heeft de applicatie daadwerkelijk gebruikt als eindgebruiker?
  • Wie heeft ook gelet op functionele vereisten die ontbreken?
"Programmers should avoid writing code that looks like dog.getBody().getTail().wag();
The solution is to rewrite our example as dog.expressHappiness();
and let the implementation of the dog decide what this means."

het 'DRY' principe

Don't
Repeat
Yourself!

Anders gezegd zijnde:
"Duplication is the root of all evil"

het 'YAGNI' principe

You
Aint
Gonna
Need
It

Anders gezegd zijnde:
Schrap "Als dit er ooit komt en in geval van x..."

Oefening - REFACTOR NA REVIEW

http://www.prato-services.eu/
vacatures@prato.be

Resources