if(a<16) {
if(p.pp) {
serP(p)
}
}
const AGE_THRESHOLD = 16;
const estimatedAge = 15;
const appearsUnderage = estimatedAge < AGE_THRESHOLD;
const hasID = !!(person.passort || person.drivingLicence);
if (appearsUnderage) {
if (hasID) {
servePerson(person)
}
}
vs
function createTicTacToeBoard() {
return ['', '', '', '', '', '', '', '', '']
}
function createTicTacToeBoard(rows=3, columns=3, startingValue='') {
return {
squares: Array.of(rows * columns).fill(startingValue),
rows: rows,
columns: columns,
};
}
vs
Don't over-think/over-engineer things! Your program should feel light and precise, not massive and overbearing!
Start off with basic stuff. Don't try to build the whole model and all its methods straight away.
Don't Repeat Yourself
button.addEventListener('click', function handler(){
doSomething(thing.property.subProperty + 1);
doSomethingElse(thing.property.subProperty * 100);
});
button.addEventListener('mouseover', function handler(){
doSomething(thing.property.subProperty + 1);
doSomethingElse(thing.property.subProperty * 100);
});
function handler(){
const value = thing.property.subProperty;
doSomething(value + 1);
doSomethingElse(value * 100);
}
button.addEventListener('click', handler);
button.addEventListener('mouseover', handler);
Bad
Good
function calculatePeriod(year, animal) {
if ((year - 1900) % 12 === 0) {
return animal[0];
} else if ((year - 1900) % 12 === 1) {
return animal[1];
} else if ((year - 1900) % 12 === 2) {
return animal[2];
} else if ((year - 1900) % 12 === 3) {
return animal[3];
} else if ((year - 1900) % 12 === 4) {
return animal[4];
} else if ((year - 1900) % 12 === 5) {
return animal[5];
} else if ((year - 1900) % 12 === 6) {
return animal[6];
} else if ((year - 1900) % 12 === 7) {
return animal[7];
} else if ((year - 1900) % 12 === 8) {
return animal[8];
} else if ((year - 1900) % 12 === 9) {
return animal[9];
} else if ((year - 1900) % 12 === 10) {
return animal[10];
} else if ((year - 1900) % 12 === 11) {
return animal[11];
}
}
Bad
const ANIMAL_CYCLE_START_YEAR = 1900;
const YEARS_IN_ONCE_CYCLE = 12;
function calculatePeriod(year) {
const yearOfTheCycle = (year - ANIMAL_CYCLE_START_YEAR) % YEARS_IN_ONCE_CYCLE;
return animals[yearOfTheCycle];
}
Good
markAsLentTo(customer) {
if (!customer instanceof Customer) { // or typeof thing !== 'string', etc.
throw new Error(
`Lending requries a customer. Received ${JSON.stringify(
customer
)} of type ${getTypeOrConstructor(customer)}`
);
}
this.custodian = customer;
}
Rule: "Every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class."
As Dr. Bob puts it: "A class[/function] should have only one reason to change"
Translation: Make your functions responsible for doing one thing. Split them down so that they can be controlled and re-used/shared more easily.
function updateUI() {
var heatingManualControl = document.getElementById("toggleHeating");
var heatingStatusDisplay = document.getElementById("heatingStatus");
var roomTempDisplay = document.getElementById("roomTempDisplay");
var checkRateDisplay = document.getElementById("checkRate");
var mercury = document.querySelector(".mercury");
var autoHeatingControl = document.getElementById("autoHeating");
heatingStatusDisplay.innerText = heatingOn ? "on" : "off";
roomTempDisplay.innerText = roomTemp;
mercury.style.height = roomTemp + "%";
checkRateDisplay.innerText = selectedCheckRate === HEATING_CHECK_INTERVAL_FAST ? 'FAST' : 'SLOW';
}
The problem here is that:
const heatingManualControl = document.getElementById("toggleHeating");
const heatingStatusDisplay = document.getElementById("heatingStatus");
const roomTempDisplay = document.getElementById("roomTempDisplay");
const checkRateDisplay = document.getElementById("checkRate");
const mercury = document.querySelector(".mercury");
const autoHeatingControl = document.getElementById("autoHeating");
// UI Actions
function updateUIHeatingStatus() {
heatingStatusDisplay.innerText = heatingOn ? "on" : "off";
}
function updateRoomTempDisplay() {
roomTempDisplay.innerText = roomTemp;
}
function updateUIThermometer() {
mercury.style.height = roomTemp + "%";
}
function updateRateDisplay() {
checkRateDisplay.innerText = selectedCheckRate === HEATING_CHECK_INTERVAL_FAST ? 'FAST' : 'SLOW';
}
function updateUI() {
updateUIThermometer();
updateRoomTempDisplay();
updateUIHeatingStatus();
updateRateDisplay();
}
Rule: "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"
Translation: You should be able to add new functionality to your code without having to re-write your existing work.
TL;DR: Provide defaults and allow for options to be passed
function Store(products=[]) {
let funds = 0;
function purchase(user, item) {
user.funds -= item.price;
// ...
}
return {
purchase: purchase
};
}
var products = [....];
var myStore = Store(products);
Problems:
function Store(products=[], funds=0, purchaseMethods={}, discounts={}) {
const defaultPurchaseMethods = {
default(){....}
};
const purchaseMethods = Object.assign({}, defaultPurchaseMethods, purchaseMethods);
function purchase(user, item, purchaseMethodName='default', discountName='') {
if(!user) throw new Error('No user provided');
if(!item) throw new Error('No item provided');
const discount = discounts[discountName] || null;
purchaseMethods[purchaseMethodName](user, item, discount);
}
return {
purchase,
};
}
// In another script:
const user = {....};
const products = [{....}, {....}];
const purchaseMethods = {....};
const discounts = {....};
const myStore = Store(products, 200, purchaseMethods, discounts);
myStore.purchase(user, 'Jumper', 'visa', 'summerSale');
Solution: Provide defaults and allow for options to be passed
Rule: "If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e. an object of type T may be substituted with any object of a subtype S) without altering any of the desirable properties of the program (correctness, task performed, etc.)"
Translation: If you override a method in the subclass, keep it predictable in what it returns or does as a side effect.
function Boat(name) {
this.name = name;
this.speed = 0;
}
Boat.prototype.go = function() {
return `${this.name} moves at ${this.speed}`; // RETURNED VALUE
}
function Powerboat() {
Boat.apply(this, arguments);
this.speed = 35;
}
Powerboat.prototype = Object.create(Boat.prototype);
Powerboat.prototype.constructor = Powerboat;
Powerboat.prototype.go = function() {
console.log(`${this.name} moves at ${this.speed}`); //NO RETURNED VALUE
}
const boats = [new Boat('Boaty'), new Powerboat('vroom')];
for (const boat of boats) {
console.log(boat.go());
}
Problem: The subclass objects go method logs the message and returns nothing, whilst the superclass returns the message.
Rule: "No client should be forced to depend on methods it does not use."
Translation: Make sure your objects are not carrying inherited or hard-coded methods they don't need [that would only be used by other objects].
function makeShape(name, width, height) {
return {
area(){ return width * height }
};
}
var rectangle = makeShape('rectangle-395', 200, 300);
Problems:
Now what about 3d shapes?
function makeShape(name, width, height, depth) {
return {
name: name,
area: function(){ return width * height },
volume: function(){ return width * height * depth },
};
}
var rectangularPrism = makeShape('rectangularPrism-203', 200, 300, 400);
var rectangle = makeShape('rectangle-395', 200, 300);
class Shape {
constructor(name='', width=0, height=0,) {
// definesive checks go here...
this.name = name;
this.width = width;
this.height = height
}
getSurfaceArea(){
return this.width * this.height
}
}
class Shape3d extends Shape {
constructor(name, width, height, depth) {
// defensive checks again...
super(name, width, height);
this.depth = depth;
}
getSurfaceArea() {
const { length:l, width:w, height:h } = this;
return 2*l*w + 2*l*h + 2*h*w;
}
getVolume() {
return width * height * depth;
}
}
const rectangle = new Shape('rectangle-395', 200, 300);
const rectangularPrism = new Shape3d('rectangularPrism-203', 200, 300, 400);
Rule:
Translation:
Don't let low-level details determine the range of high-level functional abilities. Aim for abstractions, not concretions - i.e. Lower-order functions (that do the actual work) should be as flexible and open as possible to being controlled by the higher-order functions (where the strategic decisions are made).
// Lower order function
function makeMap(pageArea, markers) {
new GoogleMap(pageArea, markers);
}
// Higher order function
function addMapsToListings(){
const mapEls = document.querySelectorAll('.map');
for (const mapEl of mapEls){
makeMap(mapEl, { .... });
});
}
Problems:
// Lower order function
function makeMap(pageArea, mapConstructor, markers) {
new mapConstructor(pageArea, markers);
}
// Higher order function
function addMapsToListings(){
const mapEls = document.querySelectorAll('.map');
for (const mapEl of mapEls) {
const mapProvider = mapEl.dataset.mapProvider; // Bing, Google, etc
const mapConstructor = mapProviders[mapProvider];
makeMap(mapEl, mapConstructor, [....]);
}
}