From STUPID to SOLID

(with a touch of IOC)

Raise your hand if you know about the STUPID practices!

Raise your hand if you know about SOLID principles!

STUPID

An acronym that defines bad practices used by programmers good and bad that promotes technical debt.

  • Singletons
  • Tight coupling
  • Untestable
  • Premature optimization
  • Indescriptive naming
  • Duplicated code

SOLID

An acronym that defines 5 principles that promote cleaner, more reusable code and generally lowers technical debt.

  • Single responsibility
  • Open/Closed
  • Liskov substitution
  • Interface segregation
  • Dependency inversion

Raise your hand if you try to avoid the STUPID practices!

Raise your hand if you try to apply the SOLID principles!

Objective

Today, we'll review some bad code that is plagued with STUPID concepts. We'll refactor that code together to get to something that is more flexible using the SOLID principles.

<?php
class TemperatureCalculator
{
	
	public static function getTemp() {

		global $sunnyDays, $snowyDays;

		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		$avg = 0;
		$cnt = 0;
		foreach($t as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	public static function getTemp() {

		return parent::getTemp() * (9 / 5) + 32 ;

	}

}

//Declared globals are affected by tempCalculator cause i'm using the temp calculator a lot
//and i don't want to change the signature
$sunnyDays = 0;
$snowyDays = 0;
if($userOptions['scale'] == 'f') {
	$avgTemp = TemperatureCalculatorFarenheit::getTemp();
} else {
	$avgTemp = TemperatureCalculator::getTemp();
}
echo 'The average temperature between december 2014 and march 2015 is '.$avgTemp.$userOptions['scale'].'<br/>';
echo 'There has been '.$sunnyDays.' days of sun and '.$snowyDays.' days of snow.';

Original code

STUPID principles

Singleton pattern

Most known and misused pattern. Singleton is mostly an anti-pattern and should be avoided.

  • Singleton should be used only to represent a resource that cannot be anything else than unique such as a physical object
     
  • Singletons introduce tight coupling in all cases and ...
     
  • Programs using the static/global state are difficult to impossible to test
  • Programs that rely on global state hide their dependencies and couple themselves in a very difficult way to fix

Did you spot a

singleton?

<?php
class TemperatureCalculator
{
	
	public static function getTemp() {

		global $sunnyDays, $snowyDays;

		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		$avg = 0;
		$cnt = 0;
		foreach($t as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	public static function getTemp() {

		return parent::getTemp() * (9 / 5) + 32 ;

	}

}

//Declared globals are affected by tempCalculator cause i'm using the temp calculator a lot
//and i don't want to change the signature
$sunnyDays = 0;
$snowyDays = 0;
if($userOptions['scale'] == 'f') {
	$avgTemp = TemperatureCalculatorFarenheit::getTemp();
} else {
	$avgTemp = TemperatureCalculator::getTemp();
}
echo 'The average temperature between december 2014 and march 2015 is '.$avgTemp.$userOptions['scale'].'<br/>';
echo 'There has been '.$sunnyDays.' days of sun and '.$snowyDays.' days of snow.';

Previous code

<?php
class TemperatureCalculator
{

	public static function getTemp() {

		global $sunnyDays, $snowyDays;

                //Get the temperature logs to work with
                $thermometherLog = new thermometerLog('south-side');
                $records = $thermometherLog->find([
                	'after' => '2014-12-21',
                	'before' => '2015-03-21'
                ]);

		$avg = 0;
		$cnt = 0;
		foreach($records as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	public static function getTemp() {

		return parent::getTemp() * (9 / 5) + 32 ;

	}

}

//Declared globals are affected by tempCalculator cause i'm using the temp calculator a lot
//and i don't want to change the signature
$sunnyDays = 0;
$snowyDays = 0;
$scale = Session::getUser()->getOption('scale');
if($scale == 'f') {
	$avgTemp = TemperatureCalculatorFarenheit::getTemp();
} else {
	$avgTemp = TemperatureCalculator::getTemp();
}
echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

New code

Tight coupling

Also known as strong coupling. Tight coupling refers to using something static or bound to a scope that is not the local scope.

  • Your code cannot adapt unless you introduce a switch or condition of some sort to create another type of object
  • Hard to test because you can't mock something created inside another object because it is created inside the class

Did you spot a

tight coupling?

<?php
class TemperatureCalculator
{

	public static function getTemp() {

		global $sunnyDays, $snowyDays;

                //Get the temperature logs to work with
                $thermometherLog = new thermometerLog('south-side');
                $records = $thermometherLog->find([
                	'after' => '2014-12-21',
                	'before' => '2015-03-21'
                ]);

		$avg = 0;
		$cnt = 0;
		foreach($records as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	public static function getTemp() {

		return parent::getTemp() * (9 / 5) + 32 ;

	}

}

//Declared globals are affected by tempCalculator cause i'm using the temp calculator a lot
//and i don't want to change the signature
$sunnyDays = 0;
$snowyDays = 0;
$scale = Session::getUser()->getOption('scale');
if($scale == 'f') {
	$avgTemp = TemperatureCalculatorFarenheit::getTemp();
} else {
	$avgTemp = TemperatureCalculator::getTemp();
}
echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

Previous code

<?php

class TemperatureCalculator
{

	/**
	 * @param TemperatureLogRecords[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		global $sunnyDays, $snowyDays;

		$avg = 0;
		$cnt = 0;
		foreach($temperatureLogRecords as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{

	/**
	 * @param TemperatureLogRecords[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

//Get the temperature logs to work with
$thermometherLog = new thermometherLog('south-side');
$t = $thermometherLog->find([
	'after' => '2014-12-21',
	'before' => '2015-03-21'
]);

//Declared globals are affected by tempCalculator cause i'm using the temp calculator a lot
//and i don't want to change the signature
$sunnyDays = 0;
$snowyDays = 0;
$scale = Session::getUser()->getOption('scale');
if($scale == 'f') {
	$avgTemp = TemperatureCalculatorFarenheit::getTemp($t);
} else {
	$avgTemp = TemperatureCalculator::getTemp($t);
}
echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

New code

<?php
class TemperatureCalculator
{

	/**
	 * @param TemperatureLogRecords[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		global $sunnyDays, $snowyDays;

		$avg = 0;
		$cnt = 0;
		foreach($temperatureLogRecords as $temp) {
			switch($temp->condition){
				case 'snowy':
					$avg += $temp->getTemp();
					$cnt++;
					$snowyDays++;
					break;
				case 'sunny':
					$avg += $temp->getTemp();
					$cnt++;
					$sunnyDays++;
					break;
			}
		}

		return $avg / $cnt;

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{

	/**
	 * @param TemperatureLogRecords[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

                //Get the temperature logs to work with
                $thermometherLog = new thermometherLog('south-side');
                $t = $thermometherLog->find([
                	'after' => '2014-12-21',
                	'before' => '2015-03-21'
                ]);

		//Declared globals are affected by tempCalculator cause i'm using the temp 
		//calculator a lot and i don't want to change the signature
		$sunnyDays = 0;
		$snowyDays = 0;
		global $sunnyDays, $snowyDays;

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$avgTemp = TemperatureCalculatorFarenheit::getTemp($t);
		} else {
			$avgTemp = TemperatureCalculator::getTemp($t);
		}
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

<?php
class TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemp(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getSunnyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getSnowyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {
		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

                //Get the temperature logs to work with
                $thermometherLog = new thermometherLog('south-side');
                $t = $thermometherLog->find([
                	'after' => '2014-12-21',
                	'before' => '2015-03-21'
                ]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$avgTemp = TemperatureCalculatorFarenheit::getTemp($t);
			$sunnyDays = TemperatureCalculatorFarenheit::getSunnyDays($t);
			$snowyDays = TemperatureCalculatorFarenheit::getSnowyDays($t);
		} else {
			$avgTemp = TemperatureCalculator::getTemp($t);
			$sunnyDays = TemperatureCalculator::getSunnyDays($t);
			$snowyDays = TemperatureCalculator::getSnowyDays($t);
		}
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

Untestable code

If code is too hard to test or untestable, you skip testability and end up creating a weak spot in your delivery.

  • Tight coupling usually creates untestable code because the coupled code cannot be replaced with a stub or a mock
  • Singletons create a state that cannot be reversed unless you use an anti-anti-pattern to reset the state...

Did you spot an

untestable portion of code?

or difficult to test code?

<?php
class TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getTemp(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemp(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getSunnyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public static function getSnowyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {
		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

                //Get the temperature logs to work with
                $thermometherLog = new thermometherLog('south-side');
                $t = $thermometherLog->find([
                	'after' => '2014-12-21',
                	'before' => '2015-03-21'
                ]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$avgTemp = TemperatureCalculatorFarenheit::getTemp($t);
			$sunnyDays = TemperatureCalculatorFarenheit::getSunnyDays($t);
			$snowyDays = TemperatureCalculatorFarenheit::getSnowyDays($t);
		} else {
			$avgTemp = TemperatureCalculator::getTemp($t);
			$sunnyDays = TemperatureCalculator::getSunnyDays($t);
			$snowyDays = TemperatureCalculator::getSnowyDays($t);
		}
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

Previous code

<?php
class TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemp(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {
		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new TemperatureCalculatorFarenheit();
		} else {
			$temperatureCalculator = new TemperatureCalculator();
		}
		$avgTemp = $temperatureCalculator->getTemp($t);
		$sunnyDays = $temperatureCalculator->getSunnyDays($t);
		$snowyDays = $temperatureCalculator->getSnowyDays($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

Premature optimization

If code isn't optimized enough, we often try to hack the code to produce something faster instead of doing what is right...

  • Using the evil "&" in foreach or in method calls
  • Using streams to read files
  • Insert any stupid code here that makes your code a little faster to run but much harder to debug

Indescriptive naming

To type less code, we often shorten our variable names or use different notations.

  • Using $i, $j, $k in a camelCaseWorld
  • Abbreviating variable names, ex: $description to $desc
  • Abbreviating function or class names

Did you spot

indescriptive namings?

<?php
class TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemp(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDays(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class TemperatureCalculatorFarenheit extends TemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getTemp(array $temperatureLogRecords) {
		return parent::getTemp($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new TemperatureCalculatorFarenheit();
		} else {
			$temperatureCalculator = new TemperatureCalculator();
		}
		$avgTemp = $temperatureCalculator->getTemp($t);
		$sunnyDays = $temperatureCalculator->getSunnyDays($t);
		$snowyDays = $temperatureCalculator->getSnowyDays($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

Previous code

<?php
class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalculator->getAverageTemperature($t);
		$sunnyDays = $temperatureCalculator->getSunnyDayCount($t);
		$snowyDays = $temperatureCalculator->getSnowyDayCount($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

Duplicated code

Duplicating code makes it easier to repeat a feature, but harder to maintain it...

  • Even if it is just a small snippet, duplication is duplication
  • Reuse by encapsulating, either in a class or in a function

Did you spot

duplicated code?

<?php
class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'sunny';
				}
			)
		);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item){ 
					return $item->condition == 'snowy';
				}
			)
		);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalculator->getAverageTemperature($t);
		$sunnyDays = $temperatureCalculator->getSunnyDayCount($t);
		$snowyDays = $temperatureCalculator->getSnowyDayCount($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

Previous code

<?php
class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'sunny');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'snowy');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 * @param string $condition
	 */
	private function countDaysBasedOnCondition(array $temperatureLogRecords, $condition) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item)use($condition){ 
					return $item->condition == $condition;
				}
			)
		);
	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalculator->getAverageTemperature($t);
		$sunnyDays = $temperatureCalculator->getSunnyDayCount($t);
		$snowyDays = $temperatureCalculator->getSnowyDayCount($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

SOLID principles

Single responsibility

Giving more than one responsibility to a class makes it harder to swap the class for another when needed.

  • It does make for more classes and interfaces (if you use them and you should) in your project, but makes it easier to navigate if well named
  • Smaller classes means easier testing

Where can you apply

the single responsiblity

principle?

<?php
class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'sunny');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'snowy');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 * @param string $condition
	 */
	private function countDaysBasedOnCondition(array $temperatureLogRecords, $condition) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item)use($condition){ 
					return $item->condition == $condition;
				}
			)
		);
	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the items based on the scale
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalculator->getAverageTemperature($t);
		$sunnyDays = $temperatureCalculator->getSunnyDayCount($t);
		$snowyDays = $temperatureCalculator->getSnowyDayCount($t);
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

Previous code

<?php
class TemperatureConditionCounter
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'sunny');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'snowy');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 * @param string $condition
	 */
	private function countDaysBasedOnCondition(array $temperatureLogRecords, $condition) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item)use($condition){ 
					return $item->condition == $condition;
				}
			)
		);
	}

}

class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalulator->getAverageTemperature($t);

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$sunnyDays = $temperatureConditionCounter->getSunnyDayCount($t);
		$snowyDays = $temperatureConditionCounter->getSnowyDayCount($t);
		
		//Output
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

New code

Open/closed principle

Every class should be open for extension and closed to modification.

  • If you change a class, you change the contract that you set up with other programmers, this is bad!
  • By opening your class to extension, you allow new functionality without changing the contract
  • Switches or if/elseif/elseif/else statements in a class usually means you have not followed this principle
  • Accepting a boolean parameter to do one thing in a case and another in another case breaks OCP too.

Where can you apply

the open/closed

principle?

<?php
class TemperatureConditionCounter
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSunnyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'sunny');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getSnowyDayCount(array $temperatureLogRecords) {
		return $this->countDaysBasedOnCondition($temperatureLogRecords, 'snowy');

	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 * @param string $condition
	 */
	private function countDaysBasedOnCondition(array $temperatureLogRecords, $condition) {
		return count(
			array_filter(
				$temperatureLogRecords, 
				function(TemperatureLogRecord $item)use($condition){ 
					return $item->condition == $condition;
				}
			)
		);
	}

}

class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalulator->getAverageTemperature($t);

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$sunnyDays = $temperatureConditionCounter->getSunnyDayCount($t);
		$snowyDays = $temperatureConditionCounter->getSnowyDayCount($t);
		
		//Output
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$sunnyDays.' sunny days and '.$snowyDays.' snowy days.';

	}

}

Previous code

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureCondition[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureCondition[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureCondition $condition
	 */
	public function addSupportedCondition(TemperatureCondition $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalulator->getAverageTemperature($t);

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('sunny'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('snowy'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		
		//Output
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

New code

Liskov substitution principle

Extending classes to retain functionality but changing it's contract is EVIL.

  • Best example: A duck and a rubber duck are not the same thing. Yes, they both quack, they both like water, but they are completely different, one is alive, the other is an inanimate object
  • Let's see if you can eat a rubber duck
  • Most of the time, the usage of EXTENDS is generally EVIL, use IMPLEMENTS instead

Where can you apply

the liskov substitution

principle?

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureCondition[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureCondition[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureCondition $condition
	 */
	public function addSupportedCondition(TemperatureCondition $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class AverageTemperatureCalculatorInFarenheit extends AverageTemperatureCalculatorInCelcius
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {
		return parent::getAverageTemperature($temperatureLogRecords) * (9 / 5) + 32 ;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$temperatureCalculator = new AverageTemperatureCalculatorInFarenheit();
		} else {
			$temperatureCalculator = new AverageTemperatureCalculatorInCelcius();
		}
		$avgTemp = $temperatureCalulator->getAverageTemperature($t);

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('sunny'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('snowy'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		
		//Output
		echo 'The temp between Dec. 2014 and Mar. 2015 is '.$avgTemp.$scale.'<br/>';
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

Previous code

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureCondition[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureCondition[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureCondition $condition
	 */
	public function addSupportedCondition(TemperatureCondition $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class FarenheitTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature * (9 / 5) + 32;

	}

}

class CelsiusTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$converter = new FarenheitTemperatureConverter();
		} else {
			$converter = new CelsiusTemperatureConverter();
		}
		$averageTemperatureCalculator = new AverageTemperatureCalculator();
		$averageTemperature = $averageTemperatureCalculator->getAverageTemperature($t);
		echo 'The average temperature is '.$converter->convert($averageTemperature).$scale.'<br />';

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('sunny'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('snowy'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

New code

Interface segregation

By asking for interfaces, you limit the reach of the code and allow easier swapping.

  • If you ask for a specific class through a type hint, you can only receive that class or extensions of that class (Remember Liskov?)
  • By using an interface, you ask only for a specific set of features that you need

Where can you apply

the interface segregation

principle?

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureCondition[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureCondition[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureCondition $condition
	 */
	public function addSupportedCondition(TemperatureCondition $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class FarenheitTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature * (9 / 5) + 32;

	}

}

class CelsiusTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$converter = new FarenheitTemperatureConverter();
		} else {
			$converter = new CelsiusTemperatureConverter();
		}
		$averageTemperatureCalculator = new AverageTemperatureCalculator();
		$averageTemperature = $averageTemperatureCalculator->getAverageTemperature($t);
		echo 'The average temperature is '.$converter->convert($averageTemperature).$scale.'<br />';

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('sunny'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition('snowy'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

Previous code

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureConditionInterface[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureConditionInterface[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureConditionInterface $condition
	 */
	public function addSupportedCondition(TemperatureConditionInterface $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class FarenheitTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature * (9 / 5) + 32;

	}

}

class CelsiusTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$converter = new FarenheitTemperatureConverter();
		} else {
			$converter = new CelsiusTemperatureConverter();
		}
		$averageTemperatureCalculator = new AverageTemperatureCalculator();
		$averageTemperature = $averageTemperatureCalculator->getAverageTemperature($t);
		echo 'The average temperature is '.$converter->convert($averageTemperature).$scale.'<br />';

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('sunny', 'sun'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('snowy', 'snow'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

New code

Dependency inversion

By asking for dependencies instead of instantiating them, you open up for extension instead of coupling up.

  • If you are creating a specific class that you need in your class, you are creating a coupling
  • Instead let the user pass you dependencies that you need
  • Interface segregation becomes important so you receive only what you need even though you may receive more you will only know of the interface

Where can you apply

the dependency inversion

principle?

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureConditionInterface[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureConditionInterface[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureConditionInterface $condition
	 */
	public function addSupportedCondition(TemperatureConditionInterface $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class FarenheitTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature * (9 / 5) + 32;

	}

}

class CelsiusTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature;

	}

}

class TemperatureController
{

	public function index() {

		//Get the temperature logs to work with
		$t = thermometerLog::get('south-side')->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = Session::getUser()->getOption('scale');
		if($scale == 'f') {
			$converter = new FarenheitTemperatureConverter();
		} else {
			$converter = new CelsiusTemperatureConverter();
		}
		$averageTemperatureCalculator = new AverageTemperatureCalculator();
		$averageTemperature = $averageTemperatureCalculator->getAverageTemperature($t);
		echo 'The average temperature is '.$converter->convert($averageTemperature).$scale.'<br />';

		//Get the condition counts
		$temperatureConditionCounter = new TemperatureConditionCounter();
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('sunny', 'sun'));
		$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('snowy', 'snow'));
		$conditions = $temperatureConditionCounter->countDaysByCondition($t);
		echo 'There has been '.$conditions['sunny'].' sunny days and '.$conditions['snowy'].' snowy days.';

	}

}

Previous code

<?php
class TemperatureConditionCounter
{

	/**
	 * @var TemperatureConditionInterface[]
	 */
	protected $supportedConditions;

	/**
	 * @return TemperatureConditionInterface[]
	 */
	public function getSupportedConditions() {
		return $this->supportedConditions;
	}

	/**
	 * @param TemperatureConditionInterface $condition
	 */
	public function addSupportedCondition(TemperatureConditionInterface $condition) {
		$this->supportedConditions[$condition->getCondition()] = $condition;
	}
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function countDaysByCondition(array $temperatureLogRecords) {
		$results = [];
		foreach($this->getSupportedConditions() as $supportedCondition) {
			$results[$supportedCondition->getCondition()] = 0;
		}
		foreach ($temperatureLogRecords as $temperatureLogRecord) {
			if (isset($this->supportedConditions[$temperatureLogRecord->getCondition()]) {
				$results[$temperatureLogRecord->getCondition()]++;
			}
		}
		return $results;
	}

}

class AverageTemperatureCalculator
{
	
	/**
	 * @param TemperatureLogRecord[] $temperatureLogRecords
	 */
	public function getAverageTemperature(array $temperatureLogRecords) {

		//Calculate the sum
		$sum = array_sum(
			array_map(
				function(TemperatureLogRecord $item){ 
					return $item->getTemperatureReading(); 
				}, 
				$temperatureLogRecords
			)
		);
		$cnt = count($temperatureLogRecords);
		return ($sum / $cnt);

	}

}

class FarenheitTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature * (9 / 5) + 32;

	}

}

class CelsiusTemperatureConverter implements CelsiusBasedTemperatureConverterInterface
{
	
	/**
	 * @param double $temperature
	 */
	public function convert($temperature) {
		return $temperature;

	}

}

class TemperatureController
{

	public function __construct(
		ThermometerLog $thermometerLog,
		SessionManagerInterface $sessionManager,
		TemperatureConversionManagerInterface $temperatureConversionManager,
		AverageTemperatureCalculatorInterface $averageTemperatureCalculator,
		TemperatureConditionCounterInterface $temperatureConditionCounter
	) {
		$this->thermometerLog = $thermometerLog;
		$this->sessionManager = $sessionManager;
		$this->temperatureConversionManager = $temperatureConversionManager;
		$this->averageTemperatureCalculator = $averageTemperatureCalculator;
		$this->temperatureConditionCounter = $temperatureConditionCounter;
	}

	public function index() {

		//Get the temperature logs to work with
		$t = $this->thermometerLog->find([
			'after' => '2014-12-21',
			'before' => '2015-03-21'
		]);

		//Get the temperature average
		$scale = $this->sessionManager->getUser()->getOption('scale');
		$converter = $this->temperatureConversionManager->getConverter($scale);
		$averageTemperature = $this->averageTemperatureCalculator->getAverageTemperature($t);
		echo 'The average temperature is '.$converter->convert($averageTemperature).$scale.'<br />';

		//Get all the different conditions instead of just sun and snow
		foreach($this->temperatureConditionCounter->getSupportedConditions() as $condition) {
			$count = $this->temperatureConditionCounter->countDaysBasedOnCondition($t, $condition);
			echo 'There has been '.$count.' days of '.$condition->label.'<br />';
		}

	}

}

New code

Inversion of control

Once you have implemented dependency inversion and interface segregation you are flooded with large constructor calls... ouch!

  • Using an IOC container will help you leverage those large constructors
  • IOC containers also help automatically adapt if new dependencies are required

Welcome to IOC

with the Illuminate Container

(Laravel)

<?php
$temperatureConversionManager = new TemperatureConversionManager();
$temperatureConversionManager->addSupportedConverter('c', new FarenheitTemperatureConverter());
$temperatureConversionManager->addSupportedConverter('f', new CelsiusTemperatureConverter());

$temperatureConditionCounter = new TemperatureConditionCounter();
$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('sunny', 'sun'));
$temperatureConditionCounter->addSupportedCondition(new TemperatureCondition2('snowy', 'snow'));

$controller = new Controller(
    ThermometerLog::get('south-side'),
    new SessionManager(),
    $temperatureConversionManager,
    new AverageTemperatureCalculator(),
    $temperatureConditionCounter
);
$controller->index();
<?php

//Bindings file
$ioc = new Illuminate\Container\Container();
$ioc->bind('ThermometerLog', function() { return ThermometerLog::get('south-side'); });
$ioc->bind('SessionManagerInterface', 'SessionManager');
$ioc->bind('TemperatureConversionManagerInterface', 'TemperatureConversionManager');
$ioc->bind('AverageTemperatureCalculatorInterface', 'AverageTemperatureCalculator');
$ioc->bind('TemperatureConditionCounterInterface', 'TemperatureConditionCounter');

$ioc->resolving('TemperatureConversionManagerInterface', function($object, $ioc) {
	$object->addSupportedConverter('c', new FarenheitTemperatureConverter());
	$object->addSupportedConverter('f', new CelsiusTemperatureConverter());
});

$ioc->resolving('TemperatureConditionCounterInterface', function($object, $ioc) {
	$object->addSupportedCondition(new TemperatureCondition2('sunny', 'sun'));
	$object->addSupportedCondition(new TemperatureCondition2('snowy', 'snow'));
});

//Front controller file
$controller = $ioc->make('TemperatureController');
$controller->index();
<?php

//Bindings file
$ioc = new Illuminate\Container\Container();
$ioc->bind('ThermometerLog', function() { return new ThermometerLog('south-side'); });
$ioc->bind('SessionManagerInterface', 'SessionManager');
$ioc->bind('TemperatureConversionManagerInterface', 'TemperatureConversionManager');
$ioc->bind('AverageTemperatureCalculatorInterface', 'AverageTemperatureCalculator');
$ioc->bind('TemperatureConditionCounterInterface', 'TemperatureConditionCounter');

$ioc->tag('FarenheitTemperatureConverter', ['TemperatureConverters']);
$ioc->tag('CelsiusTemperatureConverter', ['TemperatureConverters']);

$ioc->resolving('TemperatureConversionManagerInterface', function($object, $ioc) {
	foreach($ioc->tagged('TemperatureConverters') as $converter){
		$object->addSupportedConverter($converter->getScale(), $converter);
	}
});

$ioc->resolving('TemperatureConditionCounterInterface', function($object, $ioc) {
	$object->addSupportedCondition(new TemperatureCondition2('sunny', 'sun'));
	$object->addSupportedCondition(new TemperatureCondition2('snowy', 'snow'));
});

//Front controller file
$controller = $ioc->make('TemperatureController');
$controller->index();

Questions

From STUPID to SOLID with a touch of IOC

By Mathieu Dumoulin

From STUPID to SOLID with a touch of IOC

  • 631
Loading comments...

More from Mathieu Dumoulin