Clean code

Adapted in JavaScript

Agenda

Variables
Functions
Classes
Miscellaneous

Variables

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

Phil Karlton

Use meaningful and pronounceable variable names

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

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

Use the same vocabulary for the same type of variable

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

getUser();

Use searchable names

// What the heck is 525600 for?
for (let i = 0; i < 525600; i++) {
  runCronJob();
}

// Declare them as capitalized `const` globals.
const MINUTES_IN_A_YEAR = 525600;
for (let i = 0; i < MINUTES_IN_A_YEAR; i++) {
  runCronJob();
}

Keep convention in mind

class AnimalSoSweet {}                 // UpperCamelCase

let anAnimal = new AnimalSoSweet();    // LowerCamlCase
function sendARequest(requestToSend){};// LowerCamlCase

const NB_MS_IN_HOUR = 3600;            // SCREAMING_SNAKE_CASE

Use explanatory variables

const cityStateRegex = /^(.+)[,\\s]+(.+?)\s*(\d{5})?$/;
saveCityState(
cityStateRegex.match(cityStateRegex)[1], 
cityStateRegex.match(cityStateRegex)[2]
);

const cityStateRegex = /^(.+)[,\\s]+(.+?)\s*(\d{5})?$/;
const match = cityStateRegex.match(cityStateRegex)
const city = match[1];
const state = match[2];
saveCityState(city, state);

Avoid Mental Mapping

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

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

Don't add unneeded context

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

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

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

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

Short-circuiting is cleaner than conditionals

function createMicrobrewery(name) {
  let breweryName;
  if (name) {
    breweryName = name;
  } else {
    breweryName = 'Hipster Brew Co.';
  }
}

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

Functions

Function arguments (2 or fewer ideally)

function createMenu(title, body, buttonText, cancellable) {
  // ...
}

const menuConfig = {
  title: 'Foo',
  body: 'Bar',
  buttonText: 'Baz',
  cancellable: true
}

function createMenu(menuConfig) {
  // ...
}

Functions should do one thing (Small !)

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

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

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

Don't use flags as function parameters

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

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

function createTempFile(name) {
  createFile('./temp/' + name);
}

Function names should say what they do

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

const date = new Date();

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

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

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

Function should not have side effect

let name = 'Ryan McDermott';

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

splitIntoFirstAndLastName();

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

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

Use default arguments instead of short circuiting

function writeForumComment(subject, body) {
  subject = subject || 'No Subject';
  body = body || 'No text';
}

function writeForumComment(
    subject = 'No subject', 
    body = 'No text') {
  // ...
}

Encapsulate conditionals

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

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

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

Sort correctly callers and callee

function doVerySpecificThing(){
    //...
}

function doSpecificThing(){
    //...
    doVerySpecificThing();
}

function doThing(){
    //...
    doSpecificThing();
}

function doThing(){
    //...
    doSpecificThing();
}

function doSpecificThing(){
    //...
    doVerySpecificThing();
}

function doVerySpecificThing(){
    //...
}

Prefer functional programming over imperative

const programmerOutput = [
  { name: 'Uncle Bobby', linesOfCode: 500 },
  { name: 'Suzie Q', linesOfCode: 1500 }
  //....
];

let totalOutput = 0;

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

const programmerOutput = [
  { name: 'Uncle Bobby', linesOfCode: 500 },
  { name: 'Suzie Q', linesOfCode: 1500 }
  //....
];

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

Prefer polymorphism over lot of conditionals

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

class Airplane {}

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

class AirForceOne extends Airplane {
  getCruisingAltitude() {
    return this.getMaxAltitude();
  }
}

Never ever lets dead or commented code !!!

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

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

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

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

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

Classes

Single responsibility

Open-closed

Liskov substitution

Interface segregation

Dependency injection

SOLID principles

en.wikipedia.org/wiki/SOLID_(object-oriented_design)

Single Responsibility Principle (SRP)

class UserService {
    saveUser(user){
        eventEmmitter.dispatch(ADD_USER, user);
        return db.exec('INSERT VALUES ...', user);
    }
}

class UserResource {
    //...
    save(user){
        return db.exec('INSERT VALUES ...', user);
    }
}

class UserService {
    //...
    saveUser(user){
        eventEmmitter.dispatch(ADD_USER, user);
        return userRessource.save(user)
    }
}

"There should never be more than one reason for a class to change"

Open/Closed Principle (OCP)

class ExpenseController {
  constructor() {}

  addNewExpense(expense) {
   if (this.validateExpense(expense)){
      expenseService.save(expense);
   }
  }
  validateExpense (expense){
    return !!expense.description;
  }
}

class UserAuth {
  constructor(user) {
    this.user = user;
  }

  verifyCredentials() {
    // ...
  }
}

class ExpenseController {
  constructor(validators) {this.validators=validators;}

  addNewExpense(expense) {
   if (this.validateExpense(expense)){
      expenseService.save(expense);
   }
  }
  validateExpense (expense){
    return this.validators.every(validator => {
        return validator.isValid(expense));
    });
  }
}

"open for extension, but closed for modification"

Liskov substitution principle (LSP)

class Rectangle {
  //...
  setWidth(width) {
    this.width = width;
  }

  setHeight(height) {
    this.height = height;
  }

  getArea() {
    return this.width * this.height;
  }
}

class Square extends Rectangle {
  // ...
  setWidth(width) {
    this.width = width;
    this.height = width;
  }

  setHeight(height) {
    this.width = height;
    this.height = height;
  }
}

function renderLargeRectangles(rectangles) {
  rectangles.forEach((rectangle) => {
    rectangle.setWidth(4);
    rectangle.setHeight(5);
    const area = rectangle.getArea();//Will return 25 for Square. Should be 20.
    rectangle.render(area);
  });
}

const rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles(rectangles);

Liskov substitution principle (LSP)

class Shape {
  setColor(color) {
    // ...
  }

  render(area) {
    // ...
  }
}

class Rectangle extends Shape {
  constructor() {
    super();
    this.width = 0;
    this.height = 0;
  }

  setWidth(width) {
    this.width = width;
  }

  setHeight(height) {
    this.height = height;
  }

  getArea() {
    return this.width * this.height;
  }
}

class Square extends Shape {
  constructor() {
    super();
    this.length = 0;
  }

  setLength(length) {
    this.length = length;
  }

  getArea() {
    return this.length * this.length;
  }
}

function renderLargeShapes(shapes) {
  shapes.forEach((shape) => {
    switch (shape.constructor.name) {
      case 'Square':
        shape.setLength(5);
        break;
      case 'Rectangle':
        shape.setWidth(4);
        shape.setHeight(5);
    }

    const area = shape.getArea();
    shape.render(area);
  });
}

Interface segregation principle (ISP)

class ACar {
    run(){
        throw new Error('SubType of car should implement run');
    }
    fillFuel(){
        throw new Error('SubType of car should implement fillFuel');
    }
}
class MyOldCar extends ACar{
    run(){
        // run ...
    }
    fillFuel(){
        // handle fillFuel
    }
}
class MyElectricCar extends ACar{
    run(){
        // run ...
    }
    fillFuel(){
        // Not necessary 
    }
}

"Clients should not be forced to depend upon interfaces that they do not use."

Interface segregation principle (ISP)

class ACar {
    run(){
        throw new Error('SubType of car should implement run');
    }
}

const FuelCar = (Superclass) => class extends Superclass {
    fillFuel() {
        throw new Error('SubType of FuelCar should implement fillFuel');
    }
}
class MyOldCar extends FuelCar(ACar){
    run(){
        // run ...
    }
    fillFuel(){
        // handle fillFuel
    }
}

const ElectricCar = (Superclass) => class extends Superclass {
    reload() {
        throw new Error('SubType of ElectricCar should implement reload');
    }
}
class MyElectricCar extends ElectricCar(ACar){
    run(){
        // run ...
    }
    reload(){
        // handle reload
    }
}

"Clients should not be forced to depend upon interfaces that they do not use."

Dependency Inversion Principle (DIP)

"1. High-level modules should not depend on low-level modules. Both should depend on abstractions.

2. Abstractions should not depend upon details. Details should depend on abstractions."

class UserController {
  constructor(){
    this.userService = new UserService();
  }
  save(user){
    this.userService.save(user);
  }
}

class UserController {
  constructor(userService){
    this.userService = userService;
  }
  save(user){
    this.userService.save(user);
  }
}

Use method chaining

"1. High-level modules should not depend on low-level modules. Both should depend on abstractions.

2. Abstractions should not depend upon details. Details should depend on abstractions."

class User {
  constructor(name, age){
    this.name = name;
    this.age = age;
  }
  setName(name){
    this.name = name;
  }
  setAge(age){
    this.age = age;
  }
}

const user = new User();
user.setName('Mathieu');
user.setAge('28');

class User {
  constructor(name, age){
    this.name = name;
    this.age = age;
  }
  setName(name){
    this.name = name;
    return this;
  }
  setAge(age){
    this.age = age;
    return this;
  }
}

const user = new User()
              .setName('Mathieu')
              .setAge('28');

Miscellaneous

Only comment things that have business logic complexity.

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;
  }
}

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

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;
  }
}

Bad comments

/**
 * 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)
 */

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

while (toto < MAX_VALUES){
    if (test > MIN_VALUES){
    } // if
} // while

Follow
the clean
code rules

Clean code

Agenda

Variables

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

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

Use meaningful and pronounceable variable names

Use the same vocabulary for the same type of variable

Use searchable names

Keep convention in mind

Use explanatory variables

Avoid Mental Mapping

Don't add unneeded context

Short-circuiting is cleaner than conditionals

Functions

Function arguments (2 or fewer ideally)

Functions should do one thing (Small !)

Don't use flags as function parameters

Function names should say what they do

Function should not have side effect

Use default arguments instead of short circuiting

Encapsulate conditionals

Sort correctly callers and callee

Prefer functional programming over imperative

Prefer polymorphism over lot of conditionals

Never ever lets dead or commented code !!!

Classes

Single responsibility

Open-closed

Liskov substitution

Interface segregation

Dependency injection

SOLID principles

Single Responsibility Principle (SRP)

Open/Closed Principle (OCP)

Liskov substitution principle (LSP)

Liskov substitution principle (LSP)

Interface segregation principle (ISP)

Interface segregation principle (ISP)

Dependency Inversion Principle (DIP)

Use method chaining

Miscellaneous

Only comment things that have business logic complexity.

Bad comments

Follow the clean code rules

slides.com/mbreton/clean-code-javascript

github.com/ryanmcdermott/clean-code-javascript

Follow
the clean
code rules