FROM STUPID TO SOLID CODE

IN MODERN EXAMPLES

PRESENTED BY: TOMASZ BANASIAK

WHO AM I?

WHY DO WE NEED SOFTWARE ARCHITECTURE?

https://abstracthdwallpapers.wordpress.com/2012/07/

SOFTWARE ARCHITECTURE

  • LARGE PROJECTS

  • MANY COOPERATING TEAMS

  • NEW TEAMMATES

  • CODE RE-USE

WHAT MAKES CODE STUPID?

STUPID CODE

  • Singleton

  • Tight Coupling

  • Untestability

  • Premature Optimization

  • Indescriptive Naming

  • Duplication

STUPID ...REFACTORING

public function register() {
    $user = DBORM::factory('user');
    $user->exchangeArray($this->input->post('user', Input::TYPE_PREFIX));
    $user->save();
    $userId = $user->id;

    switch($this->input->post('type')) {
        case self::ACCOUNT_TYPE_SELLER:
          $shop = new Shop($userId, $userEmail);
          $user->addShop($shop);
          break;
        case self::ACCOUNT_TYPE_CUSTOMER:
        case self::ACCOUNT_TYPE_CLIENT:
            $dbAccounts = Module_Account_Manager::getInstance();
            $dbAccounts->create($user);
            break;
        default: 
            throw new \Exception('Unknown user type!');
            break;            
    }
    // ...
}

STUPID ...REFACTORING

public function register() {
    $user = DBORM::factory('user');
    $user->exchangeArray($this->input->post('user', Input::TYPE_PREFIX));
    $user->save();
    $userId = $user->id;
    $this->registerUserType($this->input->post('type'))
}

public function registerUserType($type) {
    switch($type) {
        case self::ACCOUNT_TYPE_SELLER:
          $shop = new Shop($userId, $userEmail);
          $user->addShop($shop);
          break;
        case self::ACCOUNT_TYPE_CUSTOMER:
        case self::ACCOUNT_TYPE_CLIENT:
            $dbAccounts = Module_Account_Manager::getInstance();
            $dbAccounts->create($user);
            break;
        // ...       
    }
    // ...
}

SOLID CODE

http://www.lifeofpix.com/

SINGLE RESPONSIBILITY PRINCIPLE

SOLID

ONE REASON TO CHANGE

OBJECT/CLASS SHOULD HAVE ONE AND 

ONLY ONE REASON TO CHANGE

https://github.com/PrestaShop/PrestaShop-1.5/blob/master/controllers/front/OrderController.php

public function processOrder()
{
  if (!Tools::getConfig('shop-shopping-available'))
    $this->context->cart->disableCart();
  
  if (!Customer::customerHasAddress($this->context->customer->id, (int)Tools::getValue('id_address_delivery'))
    || (!$same && Tools::getValue('id_address_delivery') != Tools::getValue('id_address_invoice')
      && !Customer::customerHasAddress($this->context->customer->id, (int)Tools::getValue('id_address_invoice'))))
    $this->errors[] = Tools::displayError('Invalid address', !Tools::getValue('ajax'));
  else
  {
    $this->context->cart->id_address_delivery = (int)Tools::getValue('id_address_delivery');
    $this->context->cart->id_address_invoice = (int)Tools::getValue('id_address_invoice');
    
    CartRule::lockCartProducts($this->context, strtotime('+3hours'));
    CartRule::autoAddToCart($this->context);
    
    if (!$this->context->cart->isMultiAddressDelivery())
      $this->context->cart->setNoMultishipping(); 
    
    // Add checking for all addresses
    $address_without_carriers = $this->context->cart->getDeliveryAddressesWithoutCarriers();
 
    if (count($address_without_carriers) > 1)
      $this->errors[] = sprintf(Tools::displayError('No delivery to some addresses you selected.'));
    else
      $this->errors[] = sprintf(Tools::displayError('No delivery to the address you selected.'));
    }
    foreach($this->context->cart->items as $item)
      $cartSum += $item['price'];
    // ...
  
class ProductsManager {
	public function lock(ProductsCollection $collection);	
	public function unlock(ProductsCollection $collection);
}  
  
class ProductsCollection extends Collection {
	// Collection of simple product entities (eg just ID's)
	public function sum() {};
}  

class UserAddressCollection extends Collection {
	// Collection of UserAddresses
}

class UserAdress extends Address {
	public function isOwner($userId) {};
}

class Cart {
	protected $products; // ProductsCollection
	public function preCheckout() {}
	public function postCheckout() {}
}

public function processOrder(Cart $cart, ProductsManager $productsManager)
{
  $order = new Order();
  
  try {
    $deliveryAddress = new UserAdress((int) Tools::getValue('id_address_delivery'));
    $invoiceAddress = new UserAddress((int) Tools::getValue('id_address_invoice'));
	  
    if ($deliveryAddress->isOwner($this->context->customer->id) 
      && $invoiceAddress->isOwner($this->context->customer->id) {
        $order->setAddresses($deliveryAddress, $invoiceAddress);
    }
	
    $cart->preCheckout(); // Throw an event or execute list of actions
    $productsManager->lock($cart->getProducts());  
    $order->addProducts($cart->getProducts());
    $order->save();
    $cart->postCheckout(); // Clean up cart etc	
  } catch (\Exception $e) {
	  $productsManager->unlock($cart->getProducts());
	  $this->errors[] = 'Cannot save order: ' . $e->getMessage();
  }
}

SINGLE RESPONSIBILITY - CLUES

  • MORE THAN ONE REASON TO CHANGE

  • NAMING PROBLEMS

  • MANY LOC PER METHOD/CLASS

  • HARD TO READ, HARD TO UNDERSTAND

  • NEED TO BE MODIFIED BEFORE TESTING

 

 

OPEN/CLOSED PRINCIPLE

SOLID

BE AWARE OF MY BEHAVIOUR, BE UNCONSICIOUS OF MY GEARS

OBJECT/CLASS SHOULD BE EXTENDABLE

WITHOUT INTERNAL MODIFICATIONS 

class PaymentHandler extends DefaultHandler {
    public function __construct(array $config) {
        $this->config = $config;
    }
    
    public function handlePayment(Order $order) {
        switch($order->getPayment()->getType()) {
            case self::PAYMENT_PAYPAL: 
                $this->executePaypal($order);
                break;
            // ...
        }
    }
    
    public function executePaypal(Order $order) {
        $payment = new PaypalPayment($this->config['payment']['paypal']);
        $payment->amount($order->getProducts()->getSum());
        $payment->currency($order->getPayment()->getCurrency());
        $payment->execute();
    }

    // ..
}
class PaymentHandler extends DefaultHandler {
	private $paymentHandlers;
	
	public function __construct(PaymentHandlersCollection $paymentHandlers) {
		$this->paymentHandlers = $paymentHandlers;
	}
	
	public function handlePayment(Order $order) {
		$handler = $this->paymentHandlers->get($order->getPayment()->getType());
		$handler->execute($order);
	}
}

class PaymentHandlersCollection extends Collection {
	public function get($type) {
		if (isset($this->collection[$type])) {
			return $this->collection[$type];
		} else throw new \Exception('PaymentHandler: unknown handler ' . $type);
	}
	
	public function add(PaymentHandlerInterface $paymentHandler) {
		$this->collection[] = $paymentHandler;
	}
}
class PaymentHandlerFactory extends DefaultFactory {
	public function createService(ServiceLocatorInterface $serviceLocator)
    {
		$config = $serviceLocator->get('config');
        $paymentServicePaypal = $serviceLocator->get(PaypalHandler::class);
		$paymentServicePaypal->configure($config['payment']['paypal']);
        $handlers = (new PaymentHandlersCollection())
            ->add($paymentServicePaypal);

        return new PaymentHandler($handlers);
    }
}

class PaymentInterface {
	public function configure(array $config) {}
	public function execute(Order $order) {}
}

LISKOV'S SUBSTITUTION PRINCIPLE

SOLID

TRUST ME, IM NOT WHAT IT LOOKS!

EXTENDED CLASSES SHOULD BE REPLACEABLE
BY TheIR BASE CLASSES
interface UserAwareInterface {
  /**
   * @return User
   * @throws UserNotFoundException
   */
  public function getUser();
}
class CurrentUser implements UserAwareInterface {
  public function getUser() {
    if ($this->user) {
      $user = $this->repository->getUserById($identity->userId);
      if (!$user) {
        throw new AuthException('User is not logged in or does not exist');
      }
      $this->user = $user;
    }
    return $this->user;
  }
}
class User implements UserAwareInterface {
  public function __construct($userId) {
    $this->userId = $userId;
  }

  public function getUser() {
      $user = $this->repository->getUserById($this->userId);
      return $user ? $user : null;
  }
}
class CurrentUser implements UserAwareInterface {
  public function getUser() {
    if ($this->user) {
      $user = $this->repository->getUserById($identity->userId);
      if (!$user) {
        throw new UserNotFoundException('User is not logged in or does not exist');
      }
      $this->user = $user;
    }
    return $this->user;
  }
}
class User implements UserAwareInterface {
  public function __construct($userId) {
    $this->userId = $userId;
  }

  public function getUser() {
      $user = $this->repository->getUserById($this->userId);
      if (is_null($user)) {
        throw new UserNotFoundException('Cannot find user with ID ' . $this->userId);
      } 
      return $user;
  }
}

INTERFACE SEGREGATION PRINCIPLE

SOLID

JACK OF ALL TRADES, MASTER OF NONE

IT"S BETTER TO HAVE MANY USEFUL INTERFACES INSTEAD OF COMPLICATED ONE
class DatabaseManager {
  private $databases = array();
  public function getStorage($type) {}
}
class DatabaseInterface {
  public function findById($id) {}
  public function execute($stmt) {}
  public function join(\DatabaseTable $table) {}
}
class MysqlDatabase implements DatabaseInterface {
  public function findById($id) {}
  public function execute($stmt) {}
  public function join(\DatabaseTable $table) {}
}
class MongoDatabase {
  public function findById($id) {} // works
  public function execute($stmt) {
    throw new \InvalidMethodException('Method execute does not exist in MongoDB!');
  }
  // Requires a collection instead of table
  public function join(\DatabaseTable $table) {} 
}

DEPENDENCY INVERSION PRINCIPLE

SOLID

ASK FOR INTERFACE, NOT AN IMPLEMENTATION

DEPENDENCY SHOULD RELY ON INTERFACE
NOT AN IMPLEMENTATION
class Notification {
  public function __construct(MandrillProvider $mailClient, $config) {
    $this->mailClient = $mailClient
      ->setLogin($config['login'])
      ->setSecret($config['secret'])
      ->setMessageType(MandrillClient::CONTENT_TYPE_HTML);
  }

  public function send($message, $title, $recipient) {
    $this->mailClient->send($message, $title, $recipient);
  }
}
interface Message {
  private $subject;
  private $message;
  private $type;
}

interface MailProvider {
  public function __construct($config) {}
  public function send(Message $message, Recipients $recipients) {}
}
class MandrillMail extends MailProvider {
  public function __construct($config) {
    $this->client = $mailClient
      ->setLogin($config['login'])
      ->setSecret($config['secret'])
      ->setMessageType(MandrillClient::CONTENT_TYPE_HTML);
  }

  public function send(Message $message, Recipients $recipients) {
    $this->client->send(
      $message->getMessage(), 
      $message->getTitle(), 
      $recipients->getArrayCopy()
    );
  }
}

SUMMARY

SOLID

ENTRANCE FOR WORLD OF SOFTWARE ARCHITECTURE

QUESTIONS?

SOLID

DON'T BE AFRAID TO ASK :)

INSPIRATIONS & MORE

  • Daniel Nahrebecki's Internal Presentation

  • Robert C. "Uncle Bob"  Martin's BLOG 

  • CLEAN CODE - BOOK

  • Domain Driven Design

 

THANK YOU!

TOMASZ BANASIAK

TOMASZ.BANASIAK@RST.COM.PL

HTTP://BANASIAK.PRO

RST.COM.PL/KARIERA

From STUPID to SOLID code in modern examples

By Tomasz Banasiak

Made with Slides.com

From STUPID to SOLID code in modern examples

  • 2,297

More from Tomasz Banasiak