CLEAN-CODE

VS.

DIRTY-CODE

Philippos Kardaras & Daniel Speckhardt

AGENDA

1. ABOUT US

2. NAMINGS

3. RESPONSIBILITIES

4. THE PROJECT

5. DEPLOYMENT

ABOUT PHIL

Mentality:	patient, focused and fast thinking. Likes planning stuff to the end.
Framework-Skills:	Symfony, Spryker, OXID, Typo3, EmberJs
Degree:	Diploma Informatics (University of Athens)
Career:	6 years at Turbine
Preferred food:	Steak (high quality)
Preferred drink:	Whisky

ABOUT DANIEL

Mentality:	unpatient, pragmatic, likes to get things done
Framework-Skills:	Symfony, OXID, Shopware, OroCommerce, .NET
Degree:	Master of Science in Geoinformation ( U. of Appl. Sc. Berlin)
Career	5 years at Turbine
Preferred food	Pizza (spicy)
Preferred drink	Beer

ROUND 1

NAMINGS

You should get the same meaning out of your code every time you look at it!

The compiler needs to understand the code!

doCleanUpArr()

getInfo()

fnlizeOrdr($cust, $shpOwn)

rtrveAbsDiff($f,$t) 

isOverHopped()

Some functions from DANIEL's code...

/**
This function checks an array for duplicates, 
removes them and sorts the array
*/
doCleanUpArray()

/**
This function hydrates the object-data to a structured array 
and returns the array
*/
getInfo()

/**
This function saves a order to the database,
sends email to the user and the shop-owner if input param = true
*/
fnlizeOrdr($cust, $shpOwn)

/**
This function returns the absolute difference between two numbers
*/
rtrveAbsDiff($f, $t) 

/**
Returns true when the remote server does not answer
*/
isOverHopped()

Some functions from DANIEL's code...

/**
This function checks an array for duplicates, 
removes them, sorts the array and returns it
*/
removeDuplicatesAndSort()

/**
This function hydrates the object-data to a structured array 
and returns the array
*/
getHydratedData()

/**
This function saves an order to the database,
sends email to the user and the shop-owner and returns the saved order
*/
saveOrder($sendMailToCustomer, $sendMailToShopOwner)

/**
This function returns the absolute difference between two numbers
*/
getAbsoluteDifferenceBetweenTwoNumbers($numberFrom, $numberTo) 

/**
Returns true when the remote server does not answer
*/
isServerNotReachable()

PHIL renames the functions...

removeDuplicatesAndSort()

/**
* @return array
*/
getHydratedData()

/**
* @param bool $sendMailToCustomer
* @param bool $sendMailToShopOwner
* @return OrderObject
*/
saveOrder($sendMailToCustomer, $sendMailToShopOwner)

/**
* @param int $numberFrom
* @param int $numberTo
* @return int
*/
getAbsoluteDifferenceBetweenTwoNumbers($numberFrom, $numberTo) 

/**
* @return bool
*/
isServerNotReachable()

PHIL removes comments and adds php-docs for the IDE...

removeDuplicatesAndSort(): array


getHydratedData() : array


saveOrder(
    bool $sendMailToCustomer,
    bool $sendMailToShopOwner
): OrderObject


getAbsoluteDifferenceBetweenTwoNumbers(
    int $numberFrom, 
    int $numberTo
) : int


isServerNotReachable() : bool

...or even better in PHP7: Add return types!

Phil Karlton:

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

WINNER OF ROUND NAMINGS?

ROUND 2

RESPONSIBILITIES

A baker should bake bread,

an engineer should build rockets.

I like compact code. Code splitted in too many files is unreadable.

class ContactForm
{
    public function submitForm()
    {

        $formPostData = $_POST;

        //do validation
        $isFormValid = true;
        $error = "";

        //check for
        if(empty($formPostData['email'])) {
            $error = "Email not filled";
            $isFormValid = false;
        }
        if($this->isValidEmail($formPostData['email'])) {
            $error = "Email not valid";
            $isFormValid = false;
        }
        if(empty($formPostData['firstName'])) {
            $error = "First name not filled";
            $isFormValid = false;
        }
        if(empty($formPostData['lastName'])) {
            $error = "Last name not filled";
            $isFormValid = false;
        }
        if(empty($formPostData['subject'])) {
            $error = "Subject not filled";
            $isFormValid = false;
        }
        if(empty($formPostData['content'])) {
            $error = "Content not filled";
            $isFormValid = false;
        }

        //do logging
        $this->doLogging($isFormValid, $error, $formPostData);

        if($isFormValid) {
            //send emails
            $this->sendMails($formPostData);
        }

        return $isFormValid;
    }


    private function isValidEmail($email) {
        return preg_match("/^[a-zA-Z ]*$/",$email);
    }

    private function sendMails($formPostData) {

        //-------------------- send mail to client ----------------------------
        $to      = $formPostData['email'];
        $subject = 'Thanks for your contact-request';

        $message = "Hello {$formPostData['firstName']} {$formPostData['lastName']}," . "\r\n" .
            "thanks for your request." . "\r\n\r\n" .
            "We will process your request and will contact you soon." . "\r\n\r\n" .
            "Your website team!" . "\r\n\r\n";

        $headers = 'From: owner@website.com' . "\r\n" .
            'Reply-To: owner@website.com' . "\r\n" .
            'X-Mailer: PHP/' . phpversion();

        mail($to, $subject, $message, $headers);


        //-------------------- send mail to website-owner ----------------------------
        $to      = 'owner@website.com';
        $subject = 'A new contact-form request was submitted';

        $message = "Hello website-owner," . "\r\n" .
            "First Name: {$formPostData['firstName']}" . "\r\n" .
            "Last Name: {$formPostData['lastName']}" . "\r\n" .
            "Email: {$formPostData['email']}" . "\r\n" .
            "Subject: {$formPostData['subject']}" . "\r\n" .
            "Content: {$formPostData['content']}";

        $headers = 'From: owner@website.com' . "\r\n" .
            'Reply-To: owner@website.com' . "\r\n" .
            'X-Mailer: PHP/' . phpversion();

        mail($to, $subject, $message, $headers);

    }

    private function doLogging($isFormValid, $error, $formPostData) {
        file_put_contents('./log_'.date("j.n.Y").'.txt',
            print_r($isFormValid, true) . " " . print_r($error, true) . " " . print_r($formPostData, true),
            FILE_APPEND);
    }


}

DANIEL's `ContactForm`

Uncle Bob says:

"A class should have one, and only one, reason to change."

class ContactFormHandler
{
    /** @var \Src\Form\Validator\ContactFormValidator*/
    private $contactFormValidator;

    /** @var \Src\Service\EmailService*/
    private $emailService;

    /** @var \Src\Form\Exception\Logger */
    private $logger;

    /** Order constructor. */
    public function __construct(
        ContactFormValidator $contactFormValidator,
        EmailService $emailService,
        Logger $logger
    ) {
        $this->contactFormValidator = $contactFormValidator;
        $this->emailService = $emailService;
        $this->logger = $logger;
    }
    
    /**
     * @param \App\ProductImporter\ContactForm $contactForm
     * @return bool
     */
    public function handleContactForm(ContactForm $contactForm): bool
    {
        $isValidForm = $this->contactFormValidator->validate($contactForm);

        if (!$isValidForm) {
            return false;
        }

        $emailObject = $this->emailService->buildEmail($contactForm);

        try {
            $this->emailService->sendEmail($emailObject);
            return true;
        } catch (Exception $e) {
            $this->logger->log($e);
            return false;
        }
    }
}

PHIL's `ContactFormHandler`

WINNER OF ROUND RESPONSIBILITIES?

ROUND 3

THE PROJECT

Clients Requirements

- our client runs an online shop

- we import the data to shop-database

- wants to sell products from a third party

vendor/manufacturer

- vendor uploads XML-Files to shop-server

Change Request

- client cooperates with more vendors

- vendors upload product-data as XML- or JSON-Files

another Change Request

- Shop will be open for a lot more vendors

- Check incoming data for consistency

- Business is booming!

- Make possible every format to be imported

- Has to be ready in a few days

- Implement logging

Robert Waltman says:

"The object-oriented version of 'Spaghetti code' is, of course, ..."

LASAGNA CODE!

<?php

interface HelloInterface
{
    const HELLO = 'Hello';
    public function getHello();
}

interface WorldInterface
{
    const WORLD = 'World';
	public function getWorld();
}

class Hello implements HelloInterface
{
    public function getHello()
    {
        return static::HELLO;
    }
}

class World implements WorldInterface
{
    public function getWorld()
    {
        return static::WORLD;
    }
}

trait EchoAbleTrait
{
    /**
     * @var \OutputBufferers\EchoInterface
     * @inject
     */
    protected $echoManager;

    public function echo(HelloInterface $hello, WorldInterface $world)
    {
        return $this->echoManager->createEcho(
            [
                $hello->getHello(),
                $world->getWorld(),
            ]
        );
    }
}

class HelloWorldController
{
    use EchoAbleTrait;

    public function showAction(HelloInterface $hello, WorldInterface $world)
    {
        return $this->echo($hello, $world);
    }
}

echo (new HelloWorldController)->showAction(
    new Hello(),
    new World()
);

OVERENGINEERED HELLO WORLD:

Helder Correira says:

"Keep it simple to work

but tricky to be fun"