Quality Coder

- Attitude of Professional craftmanship

 

Text

@kokkisajee

In a nutshell

Communities

Social Developer

Recognitions

  • Cloud Solution Architect @ Microsoft
  • Full-stack developer

 

I'm Sajeetharan Sinnathurai

www.sajeetharan.com

THE CHALLENGE

"Write better code for humans"

THE CHALLENGE!

and we have only 150 minutes to get there!

About this Session

👍A “crash course"

 

👍It won’t teach you everything,  But it will help you learn what you need to know

 

👍A mixture of discussion and exercises

 

👍Be prepared to talk to each other!

Who are you? 

  •  Your name
  •  Where you’re from
  •  What you do in one (short) sentence
  •  One thing you’d like to learn from this workshop

Small Activity 

#!/usr/bin/python
import sys

def p2(sum):
    ps = []
    x = 1
    while (sum > 0):
        if (sum % 2):
            ps.insert(0, x)
        x = x * 2  # Multiply by 2. Is this a bug?
        sum = sum >> 1 # Do a shift.
    # Print the powers.
    for x in ps:
        print x
    return ps

if __name__ == '__main__':
    # TODO Convert sys.arvg[1] into an integer.
    value = int(sys.argv[1])
    p2(value)

How easy it is to read?
How easy it is to understand?
How the code is structured or designed?
How the code is documented?
Anything else you think is a problem?

Bad Code Is Out To Get You!


Software Handles

  • All money except pocket change
  • Communication around the world
  • Health instruments and records
  • Almost all transportation vehicles

Software bugs costs 3.75 Trillion $

Startup Engineering

Corporate Engineering

Feature delivery flow

 

 

Someone who:

  • Knows even the dark corners of a language?
  • Knows a lot about Frameworks or technologies?
  • Writes the fastest among their co-workers?
  • Writes cryptic code that no one else can understand?         
 

What a rock-star programmer

is?

For me, a rock-star programmer is:

Someone who cares 
 for the code he writes

Leave the campground

cleaner than you found it

Use "The Boy Scout Rule":

"Any fool can write code that a computer can understand. Good programmers write code that humans can understand"

- Martin Fowler

Raise your hand if you know about SOLID principles!

Raise your hand if you know about the STUPID practices!

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.

What is Quality code?

“If you want your code to be easy to write, make it easy to read.”

"The ratio of time spent reading (code) versus writing is well over 10 to 1 ... (therefore) making it easy to read makes it easier to write."

 

- Robert C. Martin

The key for improving
the performance
of an application is:

 

Measure, measure, measure.

"Readability trumps performance" 

 

- Martin Odersky

 

If anyone can understand the code, then:

  • It reduces the amount of bugs
  • It makes code easier to test
  • It makes the code easier to fix and to extend

Why is Clean Code so important?

Features of quality code

READABILITY

Good naming of variables, functions, classes…

Good comments

Consistent coding standards

KISS

 

EXTENSIBILITY

Conscious usage of OOP principles, Design patterns

SOLID principles

 DRY

 

TESTABILITY

Unit Testing with high code coverage

Test Driven Development (TDD)

BAD CODE VS GOOD CODE

Many places to change

 Hard to find impact

More mistakes = Bugs!

Few places to change… Easy to find impact… Less mistakes!

JOURNEY TOWARDS QUALITY..

Quality should start early!

Learn by reading good code (..and bad code!)

Code reviews - give feedback, get feedback

Know your tools - IDE, Source Control, Frameworks, etc.

Passion - Love, care and be proud of your code.

Practice, practice and practice!

Source control

  • Keep track of files.
  • Collaborate on the same project.
  • Know who to blame.

Source control

  • Source control is management of software code across many developers at the same time

Branching Strategy?

Why source control?

Annotation changes - Being able to annotate 

Reversibility - Not overwriting/losing work during collaboration

Concurrency - Keeping track of all changes and being able to revert back at any point

 

Continuous Integration

What it is

What it is not

A method of finding software issues as early as possible within the development cycle and ensuring all parts of the overall platform talk to each other correctly.

Something to be ignored or bypassed because it takes effort.

Continous Integration/Deployment

“ Continuous Delivery is a way of quick and reliable switch between different versions of application in production environment"

# Tools

Jenkins

Travis

Azure Devops

Cloud build

 

Linters

"Good Practices"

# Tools

Sass, Css

Scsslint

Csslint

 

Javascript

Eslint

Jshint

Jslint

Metrics

  "Measure everything"

Tools

Codescene

Sonarqube

Veracode

CodeChef

CodeClimae

 

If tests aren't green, deploy fail

 

VARIABLES

Use meaningful and pronounceable variable names.

Bad:

const yyyymmdstr = moment().format('YYYY/MM/DD');

 

Good:

const currentDate = moment().format('YYYY/MM/DD');

INTRO TO CLEAN CODE IN JS

Use the same vocabulary for the same type of variable

Bad:

getUserInfo();
getClientData();
getCustomerRecord();

 

Good:

getUser();

INTRO TO CLEAN CODE IN JS

VARIABLES

Use searchable names

Bad:

// What the heck is 86400000 for?
setTimeout(blastOff, 86400000);

 

Good:

// Declare them as capitalized `const` globals.
const MILLISECONDS_IN_A_DAY = 86400000;

setTimeout(blastOff, MILLISECONDS_IN_A_DAY);

 

We will read more code than we will ever write. It's important that the code we do write is readable and searchable. By notnaming variables that end up being meaningful for understanding our program, we hurt our readers.  Make your names searchable.

INTRO TO CLEAN CODE IN JS

VARIABLES

Avoid Mental Mapping

Explicit is better than implicit.

Bad:

const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((l) => {
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  // Wait, what is `l` for again?
  dispatch(l);
});

INTRO TO CLEAN CODE IN JS

VARIABLES

Avoid Mental Mapping

Good:

const locations = ['Austin', 'New York', 'San Francisco'];
locations.forEach((location) => {
  doStuff();
  doSomeOtherStuff();
  // ...
  // ...
  // ...
  dispatch(location);
});

Explicit is better than implicit.

INTRO TO CLEAN CODE IN JS

VARIABLES

Don't add unneeded context

Good:

const Car = {
  make: 'Honda',
  model: 'Accord',
  color: 'Blue'
};

function paintCar(car) {
  car.color = 'Red';
}

If your class/object name tells you something, don't repeat that in your variable name.

Bad:

const Car = {
  carMake: 'Honda',
  carModel: 'Accord',
  carColor: 'Blue'
};

function paintCar(car) {
  car.carColor = 'Red';
}

INTRO TO CLEAN CODE IN JS

VARIABLES

Use default arguments instead of short circuiting or conditionals

Bad:

function createMicrobrewery(name) {
  const breweryName = name || 'Hipster Brew Co.';
  // ...
}

Good:

function createMicrobrewery(breweryName = 'Hipster Brew Co.') {
  // ...
}

INTRO TO CLEAN CODE IN JS

VARIABLES

FUNCTIONS SHOULD DO ONE THING

This is by far the most important rule in software engineering. When functions do more than one thing, they are harder to compose, test, and reason about. When you can isolate a function to just one action, they can be refactored easily and your code will read much cleaner. If you take nothing else away from this guide other than this, you'll be ahead of many developers.

FUNCTIONS

INTRO TO CLEAN CODE IN JS

Function names should say what they do

Bad:

function addToDate(date, month) {
  // ...
}

const date = new Date();

// It's hard to to tell from the function name what is added
addToDate(date, 1);

Good:

function addMonthToDate(month, date) {
  // ...
}

const date = new Date();
addMonthToDate(1, date);

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Bad:

function emailClients(clients) {
  clients.forEach((client) => {
    const clientRecord = database.lookup(client);
    if (clientRecord.isActive()) {
      email(client);
    }
  });
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Good:

function emailClients(clients) {
  clients
    .filter(isClientActive)
    .forEach(email);
}

function isClientActive(client) {
  const clientRecord = database.lookup(client);
  return clientRecord.isActive();
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

FUNCTIONS SHOULD ONLY

BE ONE LEVEL OF ABSTRACTION

When you have more than one level of abstraction your function is usually doing too much. Splitting up functions leads to reusability and easier testing.

FUNCTIONS

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Bad:

function parseBetterJSAlternative(code) {
  const REGEXES = [
    // ...
  ];

  const statements = code.split(' ');
  const tokens = [];
  REGEXES.forEach((REGEX) => {
    statements.forEach((statement) => {
      // ...
    });
  });

  const ast = [];
  tokens.forEach((token) => {
    // lex...
  });

  ast.forEach((node) => {
    // parse...
  });
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

INTRO TO CLEAN CODE IN JS

Good:

function tokenize(code) {
  const REGEXES = [
    // ...
  ];

  const statements = code.split(' ');
  const tokens = [];
  REGEXES.forEach((REGEX) => {
    statements.forEach((statement) => {
      tokens.push( /* ... */ );
    });
  });

  return tokens;
}

 
function lexer(tokens) {
  const ast = [];
  tokens.forEach((token) => {
    ast.push( /* ... */ );
  });

  return ast;
}

function parseBetterJSAlternative(code) {
  const tokens = tokenize(code);
  const ast = lexer(tokens);
  ast.forEach((node) => {
    // parse...
  });
}

Remove duplicate codes

Do your absolute best to avoid duplicate code. Duplicate code is bad because it means that there's more than one place to alter something if you need to change some logic.

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Bad:

function showDeveloperList(developers) {
  developers.forEach((developer) => {
    const expectedSalary = developer.calculateExpectedSalary();
    const experience = developer.getExperience();
    const githubLink = developer.getGithubLink();
    const data = {
      expectedSalary,
      experience,
      githubLink
    };

    render(data);
  });
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

function showManagerList(managers) {
  managers.forEach((manager) => {
    const expectedSalary = manager.calculateExpectedSalary();
    const experience = manager.getExperience();
    const portfolio = manager.getMBAProjects();
    const data = {
      expectedSalary,
      experience,
      portfolio
    };

    render(data);
  });
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Good:

function showList(employees) {
  employees.forEach((employee) => {
    const expectedSalary = employee.calculateExpectedSalary();
    const experience = employee.getExperience();

    let portfolio = employee.getGithubLink();

    if (employee.type === 'manager') {
      portfolio = employee.getMBAProjects();
    }

    const data = {
      expectedSalary,
      experience,
      portfolio
    };

    render(data);
  });
}

A DON'T USE FLAGS

AS FUNCTION PARAMETERS

FUNCTIONS

Flags tell your user that this function does more than one thing. Functions should do one thing. Split out your functions if they are following different code paths based on a boolean.

INTRO TO CLEAN CODE IN JS

Bad:

function createFile(name, temp) {
  if (temp) {
    fs.create(`./temp/${name}`);
  } else {
    fs.create(name);
  }
}

Good:

function createFile(name) {
  fs.create(name);
}

function createTempFile(name) {
  createFile(`./temp/${name}`);
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

AVOID SIDE EFFECTS (PART 1)

FUNCTIONS

A function produces a side effect if it does anything other than take a value in and return another value or values. A side effect could be writing to a file, modifying some global variable, or accidentally wiring all your money to a stranger.

INTRO TO CLEAN CODE IN JS

Bad:

// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
let name = 'Ryan McDermott';

function splitIntoFirstAndLastName() {
  name = name.split(' ');
}

splitIntoFirstAndLastName();

console.log(name); // ['Ryan', 'McDermott'];

 

Good:

function splitIntoFirstAndLastName(name) {
  return name.split(' ');
}

const name = 'Ryan McDermott';
const newName = splitIntoFirstAndLastName(name);

console.log(name); // 'Ryan McDermott';
console.log(newName); // ['Ryan', 'McDermott'];

INTRO TO CLEAN CODE IN JS

FUNCTIONS

FUNCTIONS

Favor functional programming over imperative programming

INTRO TO CLEAN CODE IN JS

Bad:

const programmerOutput = [
  {
    name: 'Uncle Bobby',
    linesOfCode: 500
  }, {
    name: 'Suzie Q',
    linesOfCode: 1500
  }, {
    name: 'Jimmy Gosling',
    linesOfCode: 150
  }, {
    name: 'Gracie Hopper',
    linesOfCode: 1000
  }
];

let totalOutput = 0;

for (let i = 0; i < programmerOutput.length; i++) {
  totalOutput += programmerOutput[i].linesOfCode;
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Good:

const programmerOutput = [
  {
    name: 'Uncle Bobby',
    linesOfCode: 500
  }, {
    name: 'Suzie Q',
    linesOfCode: 1500
  }, {
    name: 'Jimmy Gosling',
    linesOfCode: 150
  }, {
    name: 'Gracie Hopper',
    linesOfCode: 1000
  }
];

const INITIAL_VALUE = 0;

const totalOutput = programmerOutput
  .map((programmer) => programmer.linesOfCode)
  .reduce((acc, linesOfCode) => acc + linesOfCode, INITIAL_VALUE);

 

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Encapsulate conditionals

Bad:

if (fsm.state === 'fetching' && isEmpty(listNode)) {
  // ...
}

 

Good:

function shouldShowSpinner(fsm, listNode) {
  return fsm.state === 'fetching' && isEmpty(listNode);
}

if (shouldShowSpinner(fsmInstance, listNodeInstance)) {
  // ...
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

AVOID CONDITIONALS

FUNCTIONS

A function should only do one thing. When you have classes and functions that have if statements, you are telling your user that your function does more than one thing. Remember, just do one thing.

INTRO TO CLEAN CODE IN JS

Avoid negative conditionals

Bad:

function isDOMNodeNotPresent(node) {
  // ...
}

if (!isDOMNodeNotPresent(node)) {
  // ...
}


Good:

function isDOMNodePresent(node) {
  // ...
}

if (isDOMNodePresent(node)) {
  // ...
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Bad:

class Airplane {
  // ...
  getCruisingAltitude() {
    switch (this.type) {
      case '777':
        return this.getMaxAltitude() - this.getPassengerCount();
      case 'Air Force One':
        return this.getMaxAltitude();
      case 'Cessna':
        return this.getMaxAltitude() - this.getFuelExpenditure();
    }
  }
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Good:

class Airplane {
  // ...
}

class Boeing777 extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude() - this.getPassengerCount();
  }
}

class AirForceOne extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude();
  }
}
class Cessna extends Airplane {
  // ...
  getCruisingAltitude() {
    return this.getMaxAltitude() - this.getFuelExpenditure();
  }
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

DON'T OVER-OPTIMIZE

FUNCTIONS

Modern browsers do a lot of optimization under-the-hood at runtime. A lot of times, if you are optimizing then you are just wasting your time. There are good resources for seeing where optimization is lacking. Target those in the meantime, until they are fixed if they can be.

INTRO TO CLEAN CODE IN JS

Bad:

// On old browsers, each iteration with uncached `list.length` would be costly
// because of `list.length` recomputation. In modern browsers, this is optimized.
for (let i = 0, len = list.length; i < len; i++) {
  // ...
}

 

Good:

for (let i = 0; i < list.length; i++) {
  // ...
}

INTRO TO CLEAN CODE IN JS

FUNCTIONS

REMOVE DEAD CODE

Dead code is just as bad as duplicate code. There's no reason to keep it in your codebase. If it's not being called, get rid of it! It will still be safe in your version history if you still need it.

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Bad:

function oldRequestModule(url) {
  // ...
}

function newRequestModule(url) {
  // ...
}

const req = newRequestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');

Good:

function newRequestModule(url) {
  // ...
}

const req = newRequestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');

INTRO TO CLEAN CODE IN JS

FUNCTIONS

Use promises

not callbacks.

CONCURRENCY

Callbacks aren't clean, and they cause excessive amounts of nesting. With ES2015/ES6, Promises are a built-in global type. Use them!

INTRO TO CLEAN CODE IN JS

Bad:

require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (requestErr, response) => {
  if (requestErr) {
    console.error(requestErr);
  } else {
    require('fs').writeFile('article.html', response.body, (writeErr) => {
      if (writeErr) {
        console.error(writeErr);
      } else {
        console.log('File written');
      }
    });
  }
});

CONCURRENCY

INTRO TO CLEAN CODE IN JS

CONCURRENCY

Good:

require('request-promise').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin')
  .then((response) => {
    return require('fs-promise').writeFile('article.html', response);
  })
  .then(() => {
    console.log('File written');
  })
  .catch((err) => {
    console.error(err);
  });

INTRO TO CLEAN CODE IN JS

Async/Await are even cleaner than Promises.

CONCURRENCY

Promises are a very clean alternative to callbacks, but ES2017/ES8 brings async and await which offer an even cleaner solution. All you need is a function that is prefixed in an async keyword, and then you can write your logic imperatively without a then chain of functions. Use this if you can take advantage of ES2017/ES8 features today!

INTRO TO CLEAN CODE IN JS

Bad:

require('request-promise').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin')
  .then((response) => {
    return require('fs-promise').writeFile('article.html', response);
  })
  .then(() => {
    console.log('File written');
  })
  .catch((err) => {
    console.error(err);
  });

 

 

CONCURRENCY

INTRO TO CLEAN CODE IN JS

Good:

async function getCleanCodeArticle() {
  try {
    const response = await require('request-promise').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin');
    await require('fs-promise').writeFile('article.html', response);
    console.log('File written');
  } catch(err) {
    console.error(err);
  }
}

CONCURRENCY

INTRO TO CLEAN CODE IN JS

ERROR HANDLING

Thrown errors are a good thing! They mean the runtime has successfully identified when something in your program has gone wrong and it's letting you know by stopping function execution on the current stack, killing the process (in Node), and notifying you in the console with a stack trace.

DON'T IGNORE CAUGHT ERRORS

Doing nothing with a caught error doesn't give you the ability to ever fix or react to said error. Logging the error to the console (console.log) isn't much better as often times it can get lost in a sea of things printed to the console. If you wrap any bit of code in a try/catch it means you think an error may occur there and therefore you should have a plan, or create a code path, for when it occurs.

INTRO TO CLEAN CODE IN JS

Bad:

try {
  functionThatMightThrow();
} catch (error) {
  console.log(error);
}

 

Good:

try {
  functionThatMightThrow();
} catch (error) {
  // One option (more noisy than console.log):
  console.error(error);
  // Another option:
  notifyUserOfError(error);
  // Another option:
  reportErrorToService(error);
  // OR do all three!
}

INTRO TO CLEAN CODE IN JS

ERROR HANDLING

Don't ignore rejected promises

For the same reason you shouldn't ignore caught errors from try/catch.

ERROR HANDLING

Bad:

getdata()
.then((data) => {
  functionThatMightThrow(data);
})
.catch((error) => {
  console.log(error);
});

Good:

getdata()
.then((data) => {
  functionThatMightThrow(data);
})
.catch((error) => {
  // One option (more noisy than console.log):
  console.error(error);
  // Another option:
  notifyUserOfError(error);
  // Another option:
  reportErrorToService(error);
  // OR do all three!
});

INTRO TO CLEAN CODE IN JS

Formatting is subjective.

 

The main point is DO NOT ARGUE over formatting.

 

There are tons of tools to automate this. Use one!

FORMATTING

Use consistent capitalization

Capitalization tells you a lot about your variables, functions, etc. These rules are subjective, so your team can choose whatever they want. The point is, no matter what you all choose, just be consistent.

INTRO TO CLEAN CODE IN JS

FORMATTING

Bad:

const DAYS_IN_WEEK = 7;
const daysInMonth = 30;

const songs = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const Artists = ['ACDC', 'Led Zeppelin', 'The Beatles'];

function eraseDatabase() {}
function restore_database() {}

class animal {}
class Alpaca {}

 

Good:

const DAYS_IN_WEEK = 7;
const DAYS_IN_MONTH = 30;

const songs = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const artists = ['ACDC', 'Led Zeppelin', 'The Beatles'];

function eraseDatabase() {}
function restoreDatabase() {}

class Animal {}
class Alpaca {}

INTRO TO CLEAN CODE IN JS

FUNCTION CALLERS AND CALLEES SHOULD BE CLOSE

FORMATTING

If a function calls another, keep those functions vertically close in the source file. Ideally, keep the caller right above the callee. We tend to read code from top-to-bottom, like a newspaper. Because of this, make your code read that way.

INTRO TO CLEAN CODE IN JS

FORMATTING

Bad:

class PerformanceReview {
  constructor(employee) {
    this.employee = employee;
  }

  lookupPeers() {
    return db.lookup(this.employee, 'peers');
  }

  lookupManager() {
    return db.lookup(this.employee, 'manager');
  }

  getPeerReviews() {
    const peers = this.lookupPeers();
    // ...
  }

  perfReview() {
    this.getPeerReviews();
    this.getManagerReview();
    this.getSelfReview();
  }

  getManagerReview() {
    const manager = this.lookupManager();
  }

  getSelfReview() {
    // ...
  }
}

const review = new PerformanceReview(user);
review.perfReview();

INTRO TO CLEAN CODE IN JS

Good:

class PerformanceReview {
  constructor(employee) {
    this.employee = employee;
  }

  perfReview() {
    this.getPeerReviews();
    this.getManagerReview();
    this.getSelfReview();
  }

  getPeerReviews() {
    const peers = this.lookupPeers();
    // ...
  }

  lookupPeers() {
    return db.lookup(this.employee, 'peers');
  }

  getManagerReview() {
    const manager = this.lookupManager();
  }
  lookupManager() {
    return db.lookup(this.employee, 'manager');
  }

  getSelfReview() {
    // ...
  }
}

const review = new PerformanceReview(employee);
review.perfReview();

INTRO TO CLEAN CODE IN JS

FORMATTING

Comments

"Good code is its own best documentation. As you’re about to add a comment, ask yourself, ‘How can I improve the code so that this comment isn’t needed?"

- Steve McConnell

Comments

Use comments judiciously, because they get easily outdated, leading to misinformation.

Comments

If your code can only be  understood with comments, try to rewrite it.

Comments

 

  • The first line of document must be your code.

  • Don't comment code that you don't want anymore
    ( we have version control systems now )

 

Only comment things that has business logic complexity.

Comments are an apology, not a requirement. Good codes mostly documents itself.  

COMMENTS

Bad:

function hashIt(data) {
  // The hash
  let hash = 0;

  // Length of string
  const length = data.length;

  // Loop through every character in data
  for (let i = 0; i < length; i++) {
    // Get character code.
    const char = data.charCodeAt(i);
    // Make the hash
    hash = ((hash << 5) - hash) + char;
    // Convert to 32-bit integer
    hash &= hash;
  }
}

Good:

function hashIt(data) {
  let hash = 0;
  const length = data.length;

  for (let i = 0; i < length; i++) {
    const char = data.charCodeAt(i);
    hash = ((hash << 5) - hash) + char;

    // Convert to 32-bit integer
    hash &= hash;
  }
}

PRESENTED BY BIANCA GANDOLFO

INTRO TO CLEAN CODE IN JS

Don't leave commented out code in your codebase

Version control exists for a reason. Leave old code in your history.

Bad:

doStuff();
// doOtherStuff();
// doSomeMoreStuff();
// doSoMuchStuff();

Good:

doStuff();

INTRO TO CLEAN CODE IN JS

COMMENTS

Don't have journal comments.

Remember, use version control! There's no need for dead code, commented code, and especially journal comments. Use git log to get history!

Bad:

/**
 * 2016-12-20: Removed monads, didn't understand them (RM)
 * 2016-10-01: Improved using special monads (JP)
 * 2016-02-03: Removed type-checking (LI)
 * 2015-03-14: Added combine with type-checking (JR)
 */
function combine(a, b) {
  return a + b;
}

Good:

function combine(a, b) {
  return a + b;
}

INTRO TO CLEAN CODE IN JS

COMMENTS

Avoid positional markers

They usually just add noise. Let the functions and variable names along with the proper indentation and formatting give the visual structure to your code.

Bad:

///////////////////////////////////////
// Scope Model Instantiation
///////////////////////////////////////
$scope.model = {
  menu: 'foo',
  nav: 'bar'
};

///////////////////////////////////////
// Action setup
///////////////////////////////////////
const actions = function() {
  // ...
};

 

Good:

$scope.model = {
  menu: 'foo',
  nav: 'bar'
};

const actions = function() {
  // ...
};

 

INTRO TO CLEAN CODE IN JS

COMMENTS

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
<?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

Did you spot a

singleton?

<?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
<?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

Did you spot a

tight coupling?

<?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...
<?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

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
<?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

Did you spot

indescriptive namings?

<?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
<?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

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 $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

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();

Code review is a collaborative journey

Finding out a programming bug
Code formatting, linting
Discussing the specification

Sharing knowledge
Sharing skill
Making a good product through discussion

Proper Code Review

  •  Goal
  •  Background
  •  Strategy
  •  Changes made
  •  Verification steps

Ego-less Programming


Do Not Rely Solely on Senior Review

Mandatory Feynman Misquote


I couldn't explain my code to a junior. That means I don't really understand my code.

Encourage Juniors to Ask One Question


There are no stupid questions

... well, maybe there are

Massive Reduction in Bugs


Repairing a defect in acceptance test is 50 times as expensive than in requirement review


Reviews find between 51% – 70% of the defects in documents


Every hour spend in inspection saves 2,3 hours in system test


Reviews + testing can find 97% of the defects before release

Source: Ben Linders

Reviews are a process, not a destination

Prepare for multiple round-trips

Unit Testing

# Tools

Runners

Phantomjs, Karma


Test frameworks

Jasmine, Mocha, Preamble, Jest, Intern, Bootcamp(sass)


Unit Tests are great

When someone reviews them!

Test

ok,

Unit

but the

part?

What is a Unit?

If a test fails, how many things could be wrong?

The closer answer is to 1,

the more "           " the test is.

unit-y

Not Unit-y

...using a database

...talking on a networking

...needing a file

...dependent on other tests

...requiring environment config

!

External Dependencies

Congratulations

Wake up! I'M done

https://github.com/sajeetharan/TweetMe-OpenFaas-AKS

https://github.com/sajeetharan/serverless-with-openfaas-aks-workshop-colombo

Where to go from here?

- Go and Implement

- Send your queries  @kokkisajee

- Be a part of ngSrilanka

- Rate the session

  https://www.rateevent.com/speeches/SCAAA196

 

Explore by yourself and learn.

Quality Coder - FITIS workshop

By Sajeetharan Sinnathurai

Made with Slides.com

Quality Coder - FITIS workshop

Quality coder workshop organized by FITIS for industry developers

  • 999

More from Sajeetharan Sinnathurai