- Introduzione
- Variabili
- Funzioni
- Ogetti e strutture dati
- Classi
- SOLID
- Test
- Concurrency
- Error Handling
- Formatting
- Comments
- Translation
Principi di Ingegneria del Software, dal libro di Robert C. Martin Clean Code, adattati a JavaScript.
Non si tratta di una guida stilistica, bensì una guida per cercare di produrre software leggibile, riutilizzabile e rifattorizzabile in JavaScript.
Non tutti i principi di questa guida devono essere seguiti alla lettera, e solo alcuni sono universalmente condivisi. Sono linee guida e niente più, ma sono state tutte apprese in anni di esperienza collettiva dall'autore di Clean code.
Il nostro lavoro come ingegneri del software ha solo 50 anni e stiamo ancora cercando di apprendere molto. Quando l'architettura del software sarà antica come l'architettura in sè, probabilmente avremo regole più rigide da seguire. Per ora facciamo si che queste linee guida servano come termine di paragone per valutare la qualità del software che tu ed il tuo team producete.
Un ultima cosa: conoscere queste regole non farà di te immediatamente uno sviluppatore di software migliore, e lavorare per tanti anni come tale non ti eviterà di commettere errori. Ogni singola parte di codice parte come bozza, prima, per per poi prendere forma come una scultura di argilla. Solo alla fine perfezioneremo il nostro software, quando revisioneremo il codice con i nostri colleghi. Ma non ti abbattre alla prima revisione che richiederà miglioramenti: Beat up the code instead!
Da evitare
const yyyymmdstr = moment().format('YYYY/MM/DD');Bene:
const currentDate = moment().format('YYYY/MM/DD');Da evitare
getUserInfo(); getClientData(); getCustomerRecord();Bene:
getUser();Leggeremo molto più codice di quanto non ne scriveremo mai. È importante che il codice che noi scriviamo sia leggibile e ricercabile. Nominando variabili che non assumono uno specifico contesto all'interno del nostro software, irritiamo il lettore. Fai in modo che i nomi delle tue variabili siano ricercabili. Strumenti come buddy.js e ESLint possono aiutarti ad identificare costanti non rinominate.
Da evitare
// Cosa caspita significa 86400000? setTimeout(blastOff, 86400000);Bene:
// Dichiarala come costante in maiuscolo. const MILLISECONDS_IN_A_DAY = 86400000; setTimeout(blastOff, MILLISECONDS_IN_A_DAY);Da evitare
const address = 'One Infinite Loop, Cupertino 95014'; const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/; saveCityZipCode(address.match(cityZipCodeRegex)[1], address.match(cityZipCodeRegex)[2]);Bene:
const address = 'One Infinite Loop, Cupertino 95014'; const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/; const [, city, zipCode] = address.match(cityZipCodeRegex) || []; saveCityZipCode(city, zipCode);Essere espliciti è meglio che non esserlo.
Da evitare
const locations = ['Austin', 'New York', 'San Francisco']; locations.forEach((l) => { doStuff(); doSomeOtherStuff(); // ... // ... // ... // A cosa fa riferimento esattamente `l`? dispatch(l); });Bene:
const locations = ['Austin', 'New York', 'San Francisco']; locations.forEach((location) => { doStuff(); doSomeOtherStuff(); // ... // ... // ... dispatch(location); });Se il nome della tua classe/oggetto ti indica a cosa fa riferimento, non ripeterlo nei nomi delle sue proprietà o funzioni.
Da evitare
const Car = { carMake: 'Honda', carModel: 'Accord', carColor: 'Blue' }; function paintCar(car) { car.carColor = 'Red'; }Bene:
const Car = { make: 'Honda', model: 'Accord', color: 'Blue' }; function paintCar(car) { car.color = 'Red'; }nonI valori di default, generalmente sono più chiari dei cortocircuiti. Tieni presente che se non utilizzerai questo approccio, la tua funzione restituirà solo undefined come valore di default. Tutti gli altri valori "falsi" come '', "", false, null, 0, e NaN, non saranno sostituiti da un valore predefinito.
Da evitare
function createMicrobrewery(name) { const breweryName = name || 'Hipster Brew Co.'; // ... }Bene:
function createMicrobrewery(name = 'Hipster Brew Co.') { // ... }Limitare il numero di argomenti di una funzione è incredibilmente importante perchè ti permette di testarla più facilmente. Avere più di 3 argomenti può portare ad un'esplosione di combinazioni da testare, che produrranno una lunga serie di casi da verificare.
1 o 2 argomenti sono l'ideale e dovremmo evitarne un terzo se possibile. Generalmente se la tua funzione ha più di 2 argomenti, forse, sta facendo troppe operazioni. In alcuni casi, in cui questo non sia del tutto vero, un oggetto può aiutare ad ovviare a questo problema.
Dal momento in cui JavaScript permette la creazione di oggetti al volo, senza dover passare attraverso classi specifiche, puoi usare un oggetto se pensi che il tuo metodo richieda molti argomenti.
Per rendere evidente cosa la funzione si aspetta di ricevere, puoi utilizzare la sintassi destrutturata (destructuring syntax) di ES2015/ES6 che ha diversi vantaggi
To make it obvious what properties the function expects, you can use the ES2015/ES6 . This has a few advantages:
-
Quando qualcuno osserva la firma della tua funzione, è immediatamente chiaro che proprietà sono state utilizzate
-
Destrutturare, oltretutto, clona i valori primitivi passati alla funzione. Questo può prevenire effetti indesiderati. Nota: oggetti ed array destrutturati nell'oggetto usato come argomento NON saranno clonati.
-
Un Linter può avvertirti che non stai utilizzando alcune delle proprietà del tuo oggetto, non utilizzando la sintassi destrutturata non sarebbe possibile
Da evitare
function createMenu(title, body, buttonText, cancellable) { // ... }Bene:
function createMenu({ title, body, buttonText, cancellable }) { // ... } createMenu({ title: 'Foo', body: 'Bar', buttonText: 'Baz', cancellable: true });Questa è di sicuro la regola più importante nell'ingegneria del software. Quando un metodo si occupa di più di un solo aspetto sarà più difficile da testare, comporre e ragioraci sopra. Se è possibile far eseguire al metodo una sola azione sarà più facile da rifattorizzare e la leggibilità del tuo codice sarà maggiore e più chiara. Anche se non dovesse rimanerti in mente altro di questa guida, sarai comunque più avanti di molti sviluppatori.
Da evitare
function emailClients(clients) { clients.forEach((client) => { const clientRecord = database.lookup(client); if (clientRecord.isActive()) { email(client); } }); }Bene:
function emailActiveClients(clients) { clients .filter(isActiveClient) .forEach(email); } function isActiveClient(client) { const clientRecord = database.lookup(client); return clientRecord.isActive(); }Da evitare
function addToDate(date, month) { // ... } const date = new Date(); // Difficile da dire esattamente cosa viene aggiunto tramite questa funzione addToDate(date, 1);Bene:
function addMonthToDate(month, date) { // ... } const date = new Date(); addMonthToDate(1, date);Quando hai più di un livello di astrazione, la tua funzione generalmente sta facendo troppe cose. Dividere in più funzioni aiuta a riutilizzarla ed a testarla più facilmente.
Da evitare
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... }); }Bene:
function parseBetterJSAlternative(code) { const tokens = tokenize(code); const ast = lexer(tokens); ast.forEach((node) => { // parse... }); } 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; }Fai del tuo meglio per evitare codice duplicato. Duplicare il codice è un male, perchè vuol dire che c'è più di un punto da modificare nel caso in cui dovessi cambiare alcune logiche.
Immagina di avere un ristorante e di dover tener traccia del tuo magazzino: la riserva di pomodori, cipolle, aglio, spezie, etc. Se hai più di una lista in cui tieni traccia di queste quantità dovrai aggiornarle tutte, ogni volta che servirai un piatto con dei pomodori. Al contrario, se dovessi avere una sola lista, avrai un solo posto un cui dovrai tenere traccia delle modifiche sulle quantità in magazzino.
Generalmente si duplica il codice perchè ci sono due o tre piccole differenze tra una parte e l'altra del software. Questo permette di condividere le parti comuni del codice, ma allo stesso tempo avrai dei duplicati di parti che fanno la stessa cosa. Rimuovere questi duplicati, significa creare un'astrazione che permette di gestire queste differenze attraverso un unico metodo/modulo/classe.
Ottenere la sufficiente astrazione può essere complicato. Per questo dovresti seguire i principi SOLID, approfonditi nella sezione Classi. Un'astrazione non ottimale potrebbe anche essere peggio del codice duplicato, per cui fai attenzione! Non ripeterti, altrimenti dovrai aggiornare tutte le occorrenze della stessa logica ogni volta che vorrai cambiare qualcosa.
Da evitare
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); }); } 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); }); }Bene:
function showEmployeeList(employees) { employees.forEach((employee) => { const expectedSalary = employee.calculateExpectedSalary(); const experience = employee.getExperience(); const data = { expectedSalary, experience }; switch (employee.type) { case 'manager': data.portfolio = employee.getMBAProjects(); break; case 'developer': data.githubLink = employee.getGithubLink(); break; } render(data); }); }Da evitare
const menuConfig = { title: null, body: 'Bar', buttonText: null, cancellable: true }; function createMenu(config) { config.title = config.title || 'Foo'; config.body = config.body || 'Bar'; config.buttonText = config.buttonText || 'Baz'; config.cancellable = config.cancellable !== undefined ? config.cancellable : true; } createMenu(menuConfig);Bene:
const menuConfig = { title: 'Order', // User did not include 'body' key buttonText: 'Send', cancellable: true }; function createMenu(config) { config = Object.assign({ title: 'Foo', body: 'Bar', buttonText: 'Baz', cancellable: true }, config); // config now equals: {title: "Order", body: "Bar", buttonText: "Send", cancellable: true} // ... } createMenu(menuConfig);Un valore di tipo flag indica che la tua funzione può eseguire più di una sola operazione. Una funzione dovrebbe eseguire una sola operazione. Separa la tua funzione se deve eseguire più di una operazione in base al parametro flag che riceve in input.
Da evitare
function createFile(name, temp) { if (temp) { fs.create(`./temp/${name}`); } else { fs.create(name); } }Bene:
function createFile(name) { fs.create(name); } function createTempFile(name) { createFile(`./temp/${name}`); }Una funzione può generare un effetto collaterale se fa altro oltre a ricevere un valore e restituirne uno o più. L'effetto collaterale potrebbe essere scrivere su un file, modificare una variabile globale o accidentalmente girare tutti i tuoi soldi ad uno sconosciuto.
Probabilmente e occasionalmente avrai bisogno di generare un effetto collaterale: come nell'esempio precedente magari proprio scrivere su un file. Quello che dovrai fare è centralizzare il punto in cui lo fai. Non avere più funzioni e classi che fanno la stessa cosa, ma averne un servizio ed uno soltanto che se ne occupa.
La questione più importante è evitare le insidie che stanno dietro ad errate manipolazioni di oggetti senza alcuna struttura, utilizzando strutture che possono essere modificate da qualunque parte. Se riuscirai ad evitare che questo accada...sarai ben più felice della maggior parte degli altri programmatori.
Da evitare
// Variable globale utilizzata dalla funzione seguente. // Nel caso in cui dovessimo utilizzarla in un'altra funzione a questo punto si tratterebbe di un array e potrebbe generare un errore. let name = 'Ryan McDermott'; function splitIntoFirstAndLastName() { name = name.split(' '); } splitIntoFirstAndLastName(); console.log(name); // ['Ryan', 'McDermott'];Bene:
function splitIntoFirstAndLastName(name) { return name.split(' '); } const name = 'Ryan McDermott'; const newName = splitIntoFirstAndLastName(name); console.log(name); // 'Ryan McDermott'; console.log(newName); // ['Ryan', 'McDermott'];In Javascript i valori primitivi sono passati come valori, mentre oggetti ed array vengon passati come riferimento. In caso di oggetti ed array, se la tua funzione modifica l'array contenente un carrello della spesa, per esempio aggiungendo o rimuovendo un oggetto da acquistare, tutte le altre funzioni che utilizzano l'array carrello saranno condizionati da questa modifica. Queso potrebbe essere un bene ed un male allo stesso tempo. Immaginiamo una situazione in cui questo è un male:
l'utente clicca sul tasto "Acquista", che richiamerà una funzione acquista che effettua una richiesta ed invia l'array carrello al server. Per via di una pessima connessione, il metodo acquista riproverà ad effettuare la richiesta al server. Cosa succede se nello stesso momento accidentalmemte l'utente clicca su "Aggiungi al carrello" su di un oggetto che non ha intenzione di acquistare prima che venga eseguita nuovamente la funzione? Verrà inviata la richiesta con il nuovo oggetto accidentalmente aggiunto al carrello utilizzando la funzione aggiungiOggettoAlCarrello.
Un'ottima soluzione è quella di di clonare sempre l'array carrello, modificarlo e restituire il clone. Questi ci assicurerà che che nessun'altra funzione che gestisce il carrello subirà cambiamenti non voluti.
Due precisazioni vanno fatte su questo approccio:
-
Potrebbe essere che tu voglia realmente modificare l'oggetto in input, ma vedrai che utilizzando questo approccio ti accorgerai che le questi casi sono veramente rari. La maggior parte delle volte dovrai utilizzare questo approccio per non generare effetti collaterali
-
Clonare oggetti molto grandi potrebbe essere davvero dispendioso in termini di risorse. Fortunatamente questo non è un problema perchè esistono ottime librerie che permettono di utilizzare questo approccio senza dispendio di memoria e più velocemente rispetto al dovelo fare manualmente.
Da evitare
const addItemToCart = (cart, item) => { cart.push({ item, date: Date.now() }); };Bene:
const addItemToCart = (cart, item) => { return [...cart, { item, date: Date.now() }]; };Contaminare delle variabili globali è una pratica sconsigliata, in quanto potresti entrare in conflitto con altre librerie e l'utilizzatore delle tue API potrebbe non accorgersene fintanto che non si trova in produzione, generando un'eccezione. Facciamo un esempio pratico: supponiamo che tu voglia estendere il costruttore Array nativo di JavaScript aggiungendo il metodo diff che mostra le differenze tra due array. Come puoi fare? Potresti scrivere il metodo utilizzando Array.prototype, che però potrebbe entrare in conflitto con con un'altra libreria che fa la stessa cosa. Cosa succederebbe se anche l'altra libreria utilizzasse diff per trovare le differenze tra due array? Ecco perchè è molto meglio utilizzare le classi ES2015/ES6 e semplicemente estendere Array.
Da evitare
Array.prototype.diff = function diff(comparisonArray) { const hash = new Set(comparisonArray); return this.filter(elem => !hash.has(elem)); };Bene:
class SuperArray extends Array { diff(comparisonArray) { const hash = new Set(comparisonArray); return this.filter(elem => !hash.has(elem)); } }Programmazione funzionale - Programmazione imperativa
Javascript non è un linguaggio funzionale alla stregua di Haskell, ma entrambi hanno qualcosa che li accomuna. I linguaggi funzionali generalmente sono più puliti e facili da testare. Preferisci questo stile se possibile.
Da evitare
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; }Bene:
const programmerOutput = [ { name: 'Uncle Bobby', linesOfCode: 500 }, { name: 'Suzie Q', linesOfCode: 1500 }, { name: 'Jimmy Gosling', linesOfCode: 150 }, { name: 'Gracie Hopper', linesOfCode: 1000 } ]; const totalOutput = programmerOutput .map(output => output.linesOfCode) .reduce((totalLines, lines) => totalLines + lines);Da evitare
if (fsm.state === 'fetching' && isEmpty(listNode)) { // ... }Bene:
function shouldShowSpinner(fsm, listNode) { return fsm.state === 'fetching' && isEmpty(listNode); } if (shouldShowSpinner(fsmInstance, listNodeInstance)) { // ... }Da evitare
function isDOMNodeNotPresent(node) { // ... } if (!isDOMNodeNotPresent(node)) { // ... }Bene:
function isDOMNodePresent(node) { // ... } if (isDOMNodePresent(node)) { // ... }Sembrerebbe un task impossibile. Ad un rapido sguardo molti sviluppatori potrebbero pensare "come posso pensare di far funzionare qualcosa senza utilizzare un if?" La risposta è che puoi utilizzare il polimorfismo per ottenere lo stesso risultato in molti casi. La seconda domanda generalmente è "Ottimo! Ma perchè dovrei farlo?". La risposta è data in uno dei concetti precedentemente descritti: una funzione dovrebbe eseguire una sola operazione. Quando hai una Classe con delle funzioni che utilizzano lo stato if stai dicendo all'utente che la tua funzione può fare più di una operazione.
Da evitare
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(); } } }Bene:
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(); } }JavaScript è un linguaggio non tipizzato, il che significa che le tue funzioni possono accettare qualunque tipo di argomento. Qualche volta potresti essere tentato da tutta questa libertà e potresti essere altrettanto tentato di veririfcare il tipo di dato ricevuto nella tua funzione. Ci sono molti modi per evitare di dover fare questo tipo di controllo. Come prima cosa cerca scrivere API consistenti.
Da evitare
function travelToTexas(vehicle) { if (vehicle instanceof Bicycle) { vehicle.pedal(this.currentLocation, new Location('texas')); } else if (vehicle instanceof Car) { vehicle.drive(this.currentLocation, new Location('texas')); } }Bene:
function travelToTexas(vehicle) { vehicle.move(this.currentLocation, new Location('texas')); }Se stai lavorando con tipi di dati primitivi come stringhe o interi e non puoi utilizzare il paradigma del polimorfismo, ma senti ancora l'esigenza di validare il tipo di dato considera l'utilizzo di TypeScript. È una vlidissima alternativa al normale JavaScript che fornicsce una validazione di tipi statica utilizzando la sintassi JavaScript.
If you are working with basic primitive values like strings and integers, and you can't use polymorphism but you still feel the need to type-check, you should consider using TypeScript. It is an excellent alternative to normal JavaScript, as it provides you with static typing on top of standard JavaScript syntax. The problem with manually type-checking normal JavaScript is that doing it well requires so much extra verbiage that the faux "type-safety" you get doesn't make up for the lost readability. Keep your JavaScript clean, write good tests, and have good code reviews. Otherwise, do all of that but with TypeScript (which, like I said, is a great alternative!).
Da evitare
function combine(val1, val2) { if (typeof val1 === 'number' && typeof val2 === 'number' || typeof val1 === 'string' && typeof val2 === 'string') { return val1 + val2; } throw new Error('Must be of type String or Number'); }Bene:
function combine(val1, val2) { return val1 + val2; }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.
Da evitare
// 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++) { // ... }Bene:
for (let i = 0; i < list.length; i++) { // ... }Dead code is just as Male 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.
Da evitare
function oldRequestModule(url) { // ... } function newRequestModule(url) { // ... } const req = newRequestModule; inventoryTracker('apples', req, 'www.inventory-awesome.io');Bene:
function newRequestModule(url) { // ... } const req = newRequestModule; inventoryTracker('apples', req, 'www.inventory-awesome.io');Using getters and setters to access data on objects could be better than simply looking for a property on an object. "Why?" you might ask. Well, here's an unorganized list of reasons why:
- When you want to do more beyond getting an object property, you don't have to look up and change every accessor in your codebase.
- Makes adding validation simple when doing a
set. - Encapsulates the internal representation.
- Easy to add logging and error handling when getting and setting.
- You can lazy load your object's properties, let's say getting it from a server.
Da evitare
function makeBankAccount() { // ... return { balance: 0, // ... }; } const account = makeBankAccount(); account.balance = 100;Bene:
function makeBankAccount() { // this one is private let balance = 0; // a "getter", made public via the returned object below function getBalance() { return balance; } // a "setter", made public via the returned object below function setBalance(amount) { // ... validate before updating the balance balance = amount; } return { // ... getBalance, setBalance, }; } const account = makeBankAccount(); account.setBalance(100);This can be accomplished through closures (for ES5 and below).
Da evitare
const Employee = function(name) { this.name = name; }; Employee.prototype.getName = function getName() { return this.name; }; const employee = new Employee('John Doe'); console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe delete employee.name; console.log(`Employee name: ${employee.getName()}`); // Employee name: undefinedBene:
function makeEmployee(name) { return { getName() { return name; }, }; } const employee = makeEmployee('John Doe'); console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe delete employee.name; console.log(`Employee name: ${employee.getName()}`); // Employee name: John DoeIt's very difficult to get readable class inheritance, construction, and method definitions for classical ES5 Classi. If you need inheritance (and be aware that you might not), then prefer ES2015/ES6 Classi. However, prefer small funzioni over Classi until you find yourself needing larger and more complex objects.
Da evitare
const Animal = function(age) { if (!(this instanceof Animal)) { throw new Error('Instantiate Animal with `new`'); } this.age = age; }; Animal.prototype.move = function move() {}; const Mammal = function(age, furColor) { if (!(this instanceof Mammal)) { throw new Error('Instantiate Mammal with `new`'); } Animal.call(this, age); this.furColor = furColor; }; Mammal.prototype = Object.create(Animal.prototype); Mammal.prototype.constructor = Mammal; Mammal.prototype.liveBirth = function liveBirth() {}; const Human = function(age, furColor, languageSpoken) { if (!(this instanceof Human)) { throw new Error('Instantiate Human with `new`'); } Mammal.call(this, age, furColor); this.languageSpoken = languageSpoken; }; Human.prototype = Object.create(Mammal.prototype); Human.prototype.constructor = Human; Human.prototype.speak = function speak() {};Bene:
class Animal { constructor(age) { this.age = age; } move() { /* ... */ } } class Mammal extends Animal { constructor(age, furColor) { super(age); this.furColor = furColor; } liveBirth() { /* ... */ } } class Human extends Mammal { constructor(age, furColor, languageSpoken) { super(age, furColor); this.languageSpoken = languageSpoken; } speak() { /* ... */ } }This pattern is very useful in JavaScript and you see it in many libraries such as jQuery and Lodash. It allows your code to be expressive, and less verbose. For that reason, I say, use method chaining and take a look at how clean your code will be. In your class funzioni, simply return this at the end of every function, and you can chain further class methods onto it.
Da evitare
class Car { constructor(make, model, color) { this.make = make; this.model = model; this.color = color; } setMake(make) { this.make = make; } setModel(model) { this.model = model; } setColor(color) { this.color = color; } save() { console.log(this.make, this.model, this.color); } } const car = new Car('Ford','F-150','red'); car.setColor('pink'); car.save();Bene:
class Car { constructor(make, model, color) { this.make = make; this.model = model; this.color = color; } setMake(make) { this.make = make; // NOTE: Returning this for chaining return this; } setModel(model) { this.model = model; // NOTE: Returning this for chaining return this; } setColor(color) { this.color = color; // NOTE: Returning this for chaining return this; } save() { console.log(this.make, this.model, this.color); // NOTE: Returning this for chaining return this; } } const car = new Car('Ford','F-150','red') .setColor('pink') .save();As stated famously in Design Patterns by the Gang of Four, you should prefer composition over inheritance where you can. There are lots of good reasons to use inheritance and lots of good reasons to use composition. The main point for this maxim is that if your mind instinctively goes for inheritance, try to think if composition could model your problem better. In some cases it can.
You might be wondering then, "when should I use inheritance?" It depends on your problem at hand, but this is a decent list of when inheritance makes more sense than composition:
- Your inheritance represents an "is-a" relationship and not a "has-a" relationship (Human->Animal vs. User->UserDetails).
- You can reuse code from the base Classi (Humans can move like all animals).
- You want to make global changes to derived Classi by changing a base class. (Change the caloric expenditure of all animals when they move).
Da evitare
class Employee { constructor(name, email) { this.name = name; this.email = email; } // ... } // Male because Employees "have" tax data. EmployeeTaxData is not a type of Employee class EmployeeTaxData extends Employee { constructor(ssn, salary) { super(); this.ssn = ssn; this.salary = salary; } // ... }Bene:
class EmployeeTaxData { constructor(ssn, salary) { this.ssn = ssn; this.salary = salary; } // ... } class Employee { constructor(name, email) { this.name = name; this.email = email; } setTaxData(ssn, salary) { this.taxData = new EmployeeTaxData(ssn, salary); } // ... }As stated in Clean Code, "There should never be more than one reason for a class to change". It's tempting to jam-pack a class with a lot of functionality, like when you can only take one suitcase on your flight. The issue with this is that your class won't be conceptually cohesive and it will give it many reasons to change. Minimizing the amount of times you need to change a class is important. It's important because if too much functionality is in one class and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase.
Da evitare
class UserSettings { constructor(user) { this.user = user; } changeSettings(settings) { if (this.verifyCredentials()) { // ... } } verifyCredentials() { // ... } }Bene:
class UserAuth { constructor(user) { this.user = user; } verifyCredentials() { // ... } } class UserSettings { constructor(user) { this.user = user; this.auth = new UserAuth(user); } changeSettings(settings) { if (this.auth.verifyCredentials()) { // ... } } }As stated by Bertrand Meyer, "software entities (Classi, modules, funzioni, etc.) should be open for extension, but closed for modification." What does that mean though? This principle basically states that you should allow users to add new functionalities without changing existing code.
Da evitare
class AjaxAdapter extends Adapter { constructor() { super(); this.name = 'ajaxAdapter'; } } class NodeAdapter extends Adapter { constructor() { super(); this.name = 'nodeAdapter'; } } class HttpRequester { constructor(adapter) { this.adapter = adapter; } fetch(url) { if (this.adapter.name === 'ajaxAdapter') { return makeAjaxCall(url).then((response) => { // transform response and return }); } else if (this.adapter.name === 'httpNodeAdapter') { return makeHttpCall(url).then((response) => { // transform response and return }); } } } function makeAjaxCall(url) { // request and return promise } function makeHttpCall(url) { // request and return promise }Bene:
class AjaxAdapter extends Adapter { constructor() { super(); this.name = 'ajaxAdapter'; } request(url) { // request and return promise } } class NodeAdapter extends Adapter { constructor() { super(); this.name = 'nodeAdapter'; } request(url) { // request and return promise } } class HttpRequester { constructor(adapter) { this.adapter = adapter; } fetch(url) { return this.adapter.request(url).then((response) => { // transform response and return }); } }This is a scary term for a very simple concept. It's formally defined as "If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may substitute objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.)." That's an even scarier definition.
The best explanation for this is if you have a parent class and a child class, then the base class and child class can be used interchangeably without getting incorrect results. This might still be confusing, so let's take a look at the classic Square-Rectangle example. Mathematically, a square is a rectangle, but if you model it using the "is-a" relationship via inheritance, you quickly get into trouble.
Da evitare
class Rectangle { constructor() { this.width = 0; this.height = 0; } setColor(color) { // ... } render(area) { // ... } 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(); // Male: Returns 25 for Square. Should be 20. rectangle.render(area); }); } const rectangles = [new Rectangle(), new Rectangle(), new Square()]; renderLargeRectangles(rectangles);Bene:
class Shape { setColor(color) { // ... } render(area) { // ... } } class Rectangle extends Shape { constructor(width, height) { super(); this.width = width; this.height = height; } getArea() { return this.width * this.height; } } class Square extends Shape { constructor(length) { super(); this.length = length; } getArea() { return this.length * this.length; } } function renderLargeShapes(shapes) { shapes.forEach((shape) => { const area = shape.getArea(); shape.render(area); }); } const shapes = [new Rectangle(4, 5), new Rectangle(4, 5), new Square(5)]; renderLargeShapes(shapes);JavaScript doesn't have interfaces so this principle doesn't apply as strictly as others. However, it's important and relevant even with JavaScript's lack of type system.
ISP states that "Clients should not be forced to depend upon interfaces that they do not use." Interfaces are implicit contracts in JavaScript because of duck typing.
A good example to look at that demonstrates this principle in JavaScript is for Classi that require large settings objects. Not requiring clients to setup huge amounts of options is beneficial, because most of the time they won't need all of the settings. Making them optional helps prevent having a "fat interface".
Da evitare
class DOMTraverser { constructor(settings) { this.settings = settings; this.setup(); } setup() { this.rootNode = this.settings.rootNode; this.animationModule.setup(); } traverse() { // ... } } const $ = new DOMTraverser({ rootNode: document.getElementsByTagName('body'), animationModule() {} // Most of the time, we won't need to animate when traversing. // ... });Bene:
class DOMTraverser { constructor(settings) { this.settings = settings; this.options = settings.options; this.setup(); } setup() { this.rootNode = this.settings.rootNode; this.setupOptions(); } setupOptions() { if (this.options.animationModule) { // ... } } traverse() { // ... } } const $ = new DOMTraverser({ rootNode: document.getElementsByTagName('body'), options: { animationModule() {} } });This principle states two essential things:
- High-level modules should not depend on low-level modules. Both should depend on abstractions.
- Abstractions should not depend upon details. Details should depend on abstractions.
This can be hard to understand at first, but if you've worked with AngularJS, you've seen an implementation of this principle in the form of Dependency Injection (DI). While they are not identical concepts, DIP keeps high-level modules from knowing the details of its low-level modules and setting them up. It can accomplish this through DI. A huge benefit of this is that it reduces the coupling between modules. Coupling is a very Male development pattern because it makes your code hard to refactor.
As stated previously, JavaScript doesn't have interfaces so the abstractions that are depended upon are implicit contracts. That is to say, the methods and properties that an object/class exposes to another object/class. In the example below, the implicit contract is that any Request module for an InventoryTracker will have a requestItems method.
Da evitare
class InventoryRequester { constructor() { this.REQ_METHODS = ['HTTP']; } requestItem(item) { // ... } } class InventoryTracker { constructor(items) { this.items = items; // Male: We have created a dependency on a specific request implementation. // We should just have requestItems depend on a request method: `request` this.requester = new InventoryRequester(); } requestItems() { this.items.forEach((item) => { this.requester.requestItem(item); }); } } const inventoryTracker = new InventoryTracker(['apples', 'bananas']); inventoryTracker.requestItems();Bene:
class InventoryTracker { constructor(items, requester) { this.items = items; this.requester = requester; } requestItems() { this.items.forEach((item) => { this.requester.requestItem(item); }); } } class InventoryRequesterV1 { constructor() { this.REQ_METHODS = ['HTTP']; } requestItem(item) { // ... } } class InventoryRequesterV2 { constructor() { this.REQ_METHODS = ['WS']; } requestItem(item) { // ... } } // By constructing our dependencies externally and injecting them, we can easily // substitute our request module for a fancy new one that uses WebSockets. const inventoryTracker = new InventoryTracker(['apples', 'bananas'], new InventoryRequesterV2()); inventoryTracker.requestItems();Test is more important than shipping. If you have no tests or an inadequate amount, then every time you ship code you won't be sure that you didn't break anything. Deciding on what constitutes an adequate amount is up to your team, but having 100% coverage (all statements and branches) is how you achieve very high confidence and developer peace of mind. This means that in addition to having a great Test framework, you also need to use a good coverage tool.
There's no excuse to not write tests. There are plenty of good JS test frameworks, so find one that your team prefers. When you find one that works for your team, then aim to always write tests for every new feature/module you introduce. If your preferred method is Test Driven Development (TDD), that is great, but the main point is to just make sure you are reaching your coverage goals before launching any feature, or refactoring an existing one.
Da evitare
import assert from 'assert'; describe('MakeMomentJSGreatAgain', () => { it('handles date boundaries', () => { let date; date = new MakeMomentJSGreatAgain('1/1/2015'); date.addDays(30); assert.equal('1/31/2015', date); date = new MakeMomentJSGreatAgain('2/1/2016'); date.addDays(28); assert.equal('02/29/2016', date); date = new MakeMomentJSGreatAgain('2/1/2015'); date.addDays(28); assert.equal('03/01/2015', date); }); });Bene:
import assert from 'assert'; describe('MakeMomentJSGreatAgain', () => { it('handles 30-day months', () => { const date = new MakeMomentJSGreatAgain('1/1/2015'); date.addDays(30); assert.equal('1/31/2015', date); }); it('handles leap year', () => { const date = new MakeMomentJSGreatAgain('2/1/2016'); date.addDays(28); assert.equal('02/29/2016', date); }); it('handles non-leap year', () => { const date = new MakeMomentJSGreatAgain('2/1/2015'); date.addDays(28); assert.equal('03/01/2015', date); }); });Callbacks aren't clean, and they cause excessive amounts of nesting. With ES2015/ES6, Promises are a built-in global type. Use them!
Da evitare
import { get } from 'request'; import { writeFile } from 'fs'; get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (requestErr, response) => { if (requestErr) { console.error(requestErr); } else { writeFile('article.html', response.body, (writeErr) => { if (writeErr) { console.error(writeErr); } else { console.log('File written'); } }); } });Bene:
import { get } from 'request'; import { writeFile } from 'fs'; get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin') .then((response) => { return writeFile('article.html', response); }) .then(() => { console.log('File written'); }) .catch((err) => { console.error(err); });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 funzioni. Use this if you can take advantage of ES2017/ES8 features today!
Da evitare
import { get } from 'request-promise'; import { writeFile } from 'fs-promise'; get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin') .then((response) => { return writeFile('article.html', response); }) .then(() => { console.log('File written'); }) .catch((err) => { console.error(err); });Bene:
import { get } from 'request-promise'; import { writeFile } from 'fs-promise'; async function getCleanCodeArticle() { try { const response = await get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin'); await writeFile('article.html', response); console.log('File written'); } catch(err) { console.error(err); } }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.
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.
Da evitare
try { functionThatMightThrow(); } catch (error) { console.log(error); }Bene:
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! }For the same reason you shouldn't ignore caught errors from try/catch.
Da evitare
getdata() .then((data) => { functionThatMightThrow(data); }) .catch((error) => { console.log(error); });Bene:
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! });Formatting is subjective. Like many rules herein, there is no hard and fast rule that you must follow. The main point is DO NOT ARGUE over formatting. There are tons of tools to automate this. Use one! It's a waste of time and money for engineers to argue over formatting.
For things that don't fall under the purview of automatic formatting (indentation, tabs vs. spaces, double vs. single quotes, etc.) look here for some guidance.
JavaScript is untyped, so capitalization tells you a lot about your Variabili, funzioni, 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.
Da evitare
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 {}Bene:
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 {}If a function calls another, keep those funzioni 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.
Da evitare
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(employee); review.perfReview();Bene:
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();Comments are an apology, not a requirement. Good code mostly documents itself.
Da evitare
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; } }Bene:
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; } }Version control exists for a reason. Leave old code in your history.
Da evitare
doStuff(); // doOtherStuff(); // doSomeMoreStuff(); // doSoMuchStuff();Bene:
doStuff();Remember, use version control! There's no need for dead code, commented code, and especially journal comments. Use git log to get history!
Da evitare
/** * 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; }Bene:
function combine(a, b) { return a + b; }They usually just add noise. Let the funzioni and variable names along with the proper indentation and formatting give the visual structure to your code.
Da evitare
//////////////////////////////////////////////////////////////////////////////// // Scope Model Instantiation //////////////////////////////////////////////////////////////////////////////// $scope.model = { menu: 'foo', nav: 'bar' }; //////////////////////////////////////////////////////////////////////////////// // Action setup //////////////////////////////////////////////////////////////////////////////// const actions = function() { // ... };Bene:
$scope.model = { menu: 'foo', nav: 'bar' }; const actions = function() { // ... };This is also available in other languages:
Brazilian Portuguese: fesnt/clean-code-javascript
Spanish: andersontr15/clean-code-javascript
Chinese: 
German: marcbruederlin/clean-code-javascript
Korean: qkraudghgh/clean-code-javascript-ko
Polish: greg-dev/clean-code-javascript-pl
Russian: 
Vietnamese: hienvd/clean-code-javascript/
Japanese: mitsuruog/clean-code-javascript/
Indonesia: andirkh/clean-code-javascript/