From 09c2d583371cc4f52d561156c498557ffc2dd938 Mon Sep 17 00:00:00 2001 From: dancastillo Date: Mon, 12 Dec 2022 11:28:30 -0600 Subject: [PATCH] added: considerations and examples directory --- README.md | 562 ++++++++++++++++++++++++++-------------------- examples/.gitkeep | 0 2 files changed, 324 insertions(+), 238 deletions(-) create mode 100644 examples/.gitkeep diff --git a/README.md b/README.md index 342f90f..ef9ad27 100644 --- a/README.md +++ b/README.md @@ -5,18 +5,19 @@ Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-cod ## Table of Contents - 1. [Introduction](#introduction) - 2. [Variables](#variables) - 3. [Functions](#functions) - 4. [Objects and Data Structures](#objects-and-data-structures) - 5. [Classes](#classes) - 6. [SOLID](#solid) - 7. [Testing](#testing) - 8. [Concurrency](#concurrency) - 9. [Error Handling](#error-handling) - 10. [Formatting](#formatting) - 11. [Comments](#comments) - 12. [Translations](#translations) +- [Introduction](#introduction) +- [Considerations](#considerations) +- [Variables](#variables) +- [Functions](#functions) +- [Objects and Data Structures](#objects-and-data-structures) +- [Classes](#classes) +- [SOLID](#solid) +- [Testing](#testing) +- [Concurrency](#concurrency) +- [Error Handling](#error-handling) +- [Formatting](#formatting) +- [Comments](#comments) +- [Translations](#translations) ## Introduction @@ -24,14 +25,14 @@ Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-cod you shout when reading code](https://www.osnews.com/images/comics/wtfm.jpg) Software engineering principles, from Robert C. Martin's book -[*Clean Code*](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882), +[_Clean Code_](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882), adapted for TypeScript. This is not a style guide. It's a guide to producing [readable, reusable, and refactorable](https://github.com/ryanmcdermott/3rs-of-software-architecture) software in TypeScript. Not every principle herein has to be strictly followed, and even fewer will be universally agreed upon. These are guidelines and nothing more, but they are ones codified over many years of collective experience by the authors of -*Clean Code*. +_Clean Code_. Our craft of software engineering is just a bit over 50 years old, and we are still learning a lot. When software architecture is as old as architecture @@ -48,6 +49,55 @@ improvement. Beat up the code instead! **[⬆ back to top](#table-of-contents)** +## Considerations + +### Time complexity + +In order to write software that is as fast as we can make it we should take in to consideration time complexity of +the code we write. We should try to write all code under a O(n^2) complexity. + +For more information on time complexity and big O check out [here](https://www.bigocheatsheet.com/) + +**Bad:** + +```ts +for (let n = 0; i < arr.length; i++) { + for (let n = 0; i < arr.length; i++) { + for (let n = 0; i < arr.length; i++) { + // Avoid doing this loop inside a loop inside a loop. + // Time complexity is n^3 + } + } +} +``` + +**Better:** + +```ts +for (let n = 0; i < arr.length; i++) { + for (let n = 0; i < arr.length; i++) { + // Time complexity is n^2 + } +} +``` + +**Good:** + +```ts +for (let n = 0; i < arr.length; i++) { + for (let n = 0; i < arr.length; i++) { + // Time complexity is n + } +} +``` + +**Best:** + +```ts +const object = someObjectFromDataSource; +return object.valueWeAreLookingFor; +``` + ## Variables ### Use meaningful variable names @@ -60,7 +110,6 @@ Distinguish names in such a way that the reader knows what the differences offer function between(a1: T, a2: T, a3: T): boolean { return a2 <= a1 && a1 <= a3; } - ``` **Good:** @@ -84,7 +133,7 @@ type DtaRcrd102 = { genymdhms: Date; modymdhms: Date; pszqint: number; -} +}; ``` **Good:** @@ -94,7 +143,7 @@ type Customer = { generationTimestamp: Date; modificationTimestamp: Date; recordId: number; -} +}; ``` **[⬆ back to top](#table-of-contents)** @@ -119,7 +168,7 @@ function getUser(): User; ### Use searchable names -We will read more code than we will ever write. It's important that the code we do write must be readable and searchable. By *not* naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like [ESLint](https://typescript-eslint.io/) can help identify unnamed constants. +We will read more code than we will ever write. It's important that the code we do write must be readable and searchable. By _not_ naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like [ESLint](https://typescript-eslint.io/) can help identify unnamed constants. **Bad:** @@ -166,7 +215,7 @@ for (const [id, user] of users) { ### Avoid Mental Mapping Explicit is better than implicit. -*Clarity is king.* +_Clarity is king._ **Bad:** @@ -197,7 +246,7 @@ type Car = { carMake: string; carModel: string; carColor: string; -} +}; function print(car: Car): void { console.log(`${car.carMake} ${car.carModel} (${car.carColor})`); @@ -211,7 +260,7 @@ type Car = { make: string; model: string; color: string; -} +}; function print(car: Car): void { console.log(`${car.make} ${car.model} (${car.color})`); @@ -256,7 +305,7 @@ const GENRE = { DRAMA: 'drama', COMEDY: 'comedy', DOCUMENTARY: 'documentary', -} +}; projector.configureFilm(GENRE.COMEDY); @@ -265,7 +314,7 @@ class Projector { configureFilm(genre) { switch (genre) { case GENRE.ROMANTIC: - // some logic to be executed + // some logic to be executed } } } @@ -288,7 +337,7 @@ class Projector { configureFilm(genre) { switch (genre) { case GENRE.ROMANTIC: - // some logic to be executed + // some logic to be executed } } } @@ -301,13 +350,13 @@ class Projector { ### Function arguments (2 or fewer ideally) Limiting the number of function parameters is incredibly important because it makes testing your function easier. -Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument. +Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument. One or two arguments is the ideal case, and three should be avoided if possible. Anything more than that should be consolidated. Usually, if you have more than two arguments then your function is trying to do too much. -In cases where it's not, most of the time a higher-level object will suffice as an argument. +In cases where it's not, most of the time a higher-level object will suffice as an argument. -Consider using object literals if you are finding yourself needing a lot of arguments. +Consider using object literals if you are finding yourself needing a lot of arguments. To make it obvious what properties the function expects, you can use the [destructuring](https://basarat.gitbook.io/typescript/future-javascript/destructuring) syntax. This has a few advantages: @@ -333,7 +382,7 @@ createMenu('Foo', 'Bar', 'Baz', true); **Good:** ```ts -function createMenu(options: { title: string, body: string, buttonText: string, cancellable: boolean }) { +function createMenu(options: { title: string; body: string; buttonText: string; cancellable: boolean }) { // ... } @@ -341,15 +390,14 @@ createMenu({ title: 'Foo', body: 'Bar', buttonText: 'Baz', - cancellable: true + cancellable: true, }); ``` You can further improve readability by using [type aliases](https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-aliases): ```ts - -type MenuOptions = { title: string, body: string, buttonText: string, cancellable: boolean }; +type MenuOptions = { title: string; body: string; buttonText: string; cancellable: boolean }; function createMenu(options: MenuOptions) { // ... @@ -359,7 +407,7 @@ createMenu({ title: 'Foo', body: 'Bar', buttonText: 'Baz', - cancellable: true + cancellable: true, }); ``` @@ -433,7 +481,9 @@ When you have more than one level of abstraction your function is usually doing ```ts function parseCode(code: string) { - const REGEXES = [ /* ... */ ]; + const REGEXES = [ + /* ... */ + ]; const statements = code.split(' '); const tokens = []; @@ -457,7 +507,9 @@ function parseCode(code: string) { **Good:** ```ts -const REGEXES = [ /* ... */ ]; +const REGEXES = [ + /* ... */ +]; function parseCode(code: string) { const tokens = tokenize(code); @@ -474,7 +526,7 @@ function tokenize(code: string): Token[] { REGEXES.forEach((regex) => { statements.forEach((statement) => { - tokens.push( /* ... */ ); + tokens.push(/* ... */); }); }); @@ -484,7 +536,7 @@ function tokenize(code: string): Token[] { function parse(tokens: Token[]): SyntaxTree { const syntaxTree: SyntaxTree[] = []; tokens.forEach((token) => { - syntaxTree.push( /* ... */ ); + syntaxTree.push(/* ... */); }); return syntaxTree; @@ -496,13 +548,13 @@ function parse(tokens: Token[]): SyntaxTree { ### Remove duplicate code 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. +Duplicate code is bad because it means that there's more than one place to alter something if you need to change some logic. Imagine if you run a restaurant and you keep track of your inventory: all your tomatoes, onions, garlic, spices, etc. If you have multiple lists that you keep this on, then all have to be updated when you serve a dish with tomatoes in them. -If you only have one list, there's only one place to update! +If you only have one list, there's only one place to update! -Oftentimes you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you to have two or more separate functions that do much of the same things. Removing duplicate code means creating an abstraction that can handle this set of different things with just one function/module/class. +Oftentimes you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you to have two or more separate functions that do much of the same things. Removing duplicate code means creating an abstraction that can handle this set of different things with just one function/module/class. Getting the abstraction right is critical, that's why you should follow the [SOLID](#solid) principles. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make a good abstraction, do it! Don't repeat yourself, otherwise, you'll find yourself updating multiple places anytime you want to change one thing. @@ -518,7 +570,7 @@ function showDeveloperList(developers: Developer[]) { const data = { expectedSalary, experience, - githubLink + githubLink, }; render(data); @@ -534,7 +586,7 @@ function showManagerList(managers: Manager[]) { const data = { expectedSalary, experience, - portfolio + portfolio, }; render(data); @@ -550,7 +602,7 @@ class Developer { getExtraDetails() { return { githubLink: this.githubLink, - } + }; } } @@ -559,7 +611,7 @@ class Manager { getExtraDetails() { return { portfolio: this.portfolio, - } + }; } } @@ -581,6 +633,7 @@ function showEmployeeList(employee: (Developer | Manager)[]) { ``` You may also consider adding a union type, or common parent class if it suits your abstraction. + ```ts class Developer { // ... @@ -608,7 +661,7 @@ You should be critical about code duplication. Sometimes there is a tradeoff bet **Bad:** ```ts -type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean }; +type MenuConfig = { title?: string; body?: string; buttonText?: string; cancellable?: boolean }; function createMenu(config: MenuConfig) { config.title = config.title || 'Foo'; @@ -625,15 +678,18 @@ createMenu({ body: 'Bar' }); **Good:** ```ts -type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean }; +type MenuConfig = { title?: string; body?: string; buttonText?: string; cancellable?: boolean }; function createMenu(config: MenuConfig) { - const menuConfig = Object.assign({ - title: 'Foo', - body: 'Bar', - buttonText: 'Baz', - cancellable: true - }, config); + const menuConfig = Object.assign( + { + title: 'Foo', + body: 'Bar', + buttonText: 'Baz', + cancellable: true, + }, + config, + ); // ... } @@ -656,13 +712,14 @@ function createMenu(config: MenuConfig) { // ... } ``` + The spread operator and `Object.assign()` are very similar. The main difference is that spreading defines new properties, while `Object.assign()` sets them. More detailed, the difference is explained in [this](https://stackoverflow.com/questions/32925460/object-spread-vs-object-assign) thread. Alternatively, you can use destructuring with default values: ```ts -type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean }; +type MenuConfig = { title?: string; body?: string; buttonText?: string; cancellable?: boolean }; function createMenu({ title = 'Foo', body = 'Bar', buttonText = 'Baz', cancellable = true }: MenuConfig) { // ... @@ -710,11 +767,11 @@ function createFile(name: string) { ### Avoid Side Effects (part 1) 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. +A side effect could be writing to a file, modifying some global variable, or accidentally wiring all your money to a stranger. Now, you do need to have side effects in a program on occasion. Like the previous example, you might need to write to a file. What you want to do is to centralize where you are doing this. Don't have several functions and classes that write to a particular file. -Have one service that does it. One and only one. +Have one service that does it. One and only one. The main point is to avoid common pitfalls like sharing state between objects without any structure, using mutable data types that can be written to by anything, and not centralizing where your side effects occur. If you can do this, you will be happier than the vast majority of other programmers. @@ -751,7 +808,7 @@ console.log(name); ### Avoid Side Effects (part 2) -Browsers and Node.js process only JavaScript, therefore any TypeScript code has to be compiled before running or debugging. In JavaScript, some values are unchangeable (immutable) and some are changeable (mutable). Objects and arrays are two kinds of mutable values so it's important to handle them carefully when they're passed as parameters to a function. A JavaScript function can change an object's properties or alter the contents of an array which could easily cause bugs elsewhere. +Browsers and Node.js process only JavaScript, therefore any TypeScript code has to be compiled before running or debugging. In JavaScript, some values are unchangeable (immutable) and some are changeable (mutable). Objects and arrays are two kinds of mutable values so it's important to handle them carefully when they're passed as parameters to a function. A JavaScript function can change an object's properties or alter the contents of an array which could easily cause bugs elsewhere. Suppose there's a function that accepts an array parameter representing a shopping cart. If the function makes a change in that shopping cart array - by adding an item to purchase, for example - then any other function that uses that same `cart` array will be affected by this addition. That may be great, however it could also be bad. Let's imagine a bad situation: @@ -770,7 +827,7 @@ Two caveats to mention to this approach: ```ts function addItemToCart(cart: CartItem[], item: Item): void { cart.push({ item, date: Date.now() }); -}; +} ``` **Good:** @@ -778,7 +835,7 @@ function addItemToCart(cart: CartItem[], item: Item): void { ```ts function addItemToCart(cart: CartItem[], item: Item): CartItem[] { return [...cart, { item, date: Date.now() }]; -}; +} ``` **[⬆ back to top](#table-of-contents)** @@ -799,7 +856,7 @@ declare global { if (!Array.prototype.diff) { Array.prototype.diff = function (other: T[]): T[] { const hash = new Set(other); - return this.filter(elem => !hash.has(elem)); + return this.filter((elem) => !hash.has(elem)); }; } ``` @@ -810,8 +867,8 @@ if (!Array.prototype.diff) { class MyArray extends Array { diff(other: T[]): T[] { const hash = new Set(other); - return this.filter(elem => !hash.has(elem)); - }; + return this.filter((elem) => !hash.has(elem)); + } } ``` @@ -827,17 +884,20 @@ Favor this style of programming when you can. const contributions = [ { name: 'Uncle Bobby', - linesOfCode: 500 - }, { + linesOfCode: 500, + }, + { name: 'Suzie Q', - linesOfCode: 1500 - }, { + linesOfCode: 1500, + }, + { name: 'Jimmy Gosling', - linesOfCode: 150 - }, { + linesOfCode: 150, + }, + { name: 'Gracie Hopper', - linesOfCode: 1000 - } + linesOfCode: 1000, + }, ]; let totalOutput = 0; @@ -853,21 +913,23 @@ for (let i = 0; i < contributions.length; i++) { const contributions = [ { name: 'Uncle Bobby', - linesOfCode: 500 - }, { + linesOfCode: 500, + }, + { name: 'Suzie Q', - linesOfCode: 1500 - }, { + linesOfCode: 1500, + }, + { name: 'Jimmy Gosling', - linesOfCode: 150 - }, { + linesOfCode: 150, + }, + { name: 'Gracie Hopper', - linesOfCode: 1000 - } + linesOfCode: 1000, + }, ]; -const totalOutput = contributions - .reduce((totalLines, output) => totalLines + output.linesOfCode, 0); +const totalOutput = contributions.reduce((totalLines, output) => totalLines + output.linesOfCode, 0); ``` **[⬆ back to top](#table-of-contents)** @@ -1082,7 +1144,7 @@ Use generators and iterables when working with collections of data used like a s There are some good reasons: - decouples the callee from the generator implementation in a sense that callee decides how many -items to access + items to access - lazy execution, items are streamed on-demand - built-in support for iterating items using the `for-of` syntax - iterables allow implementing optimized iterator patterns @@ -1103,7 +1165,7 @@ function fibonacci(n: number): number[] { } function print(n: number) { - fibonacci(n).forEach(fib => console.log(fib)); + fibonacci(n).forEach((fib) => console.log(fib)); } // Print first 10 Fibonacci numbers. @@ -1127,9 +1189,9 @@ function* fibonacci(): IterableIterator { function print(n: number) { let i = 0; for (const fib of fibonacci()) { - if (i++ === n) break; + if (i++ === n) break; console.log(fib); - } + } } // Print first 10 Fibonacci numbers. @@ -1145,7 +1207,7 @@ import itiriri from 'itiriri'; function* fibonacci(): IterableIterator { let [a, b] = [0, 1]; - + while (true) { yield a; [a, b] = [b, a + b]; @@ -1154,7 +1216,7 @@ function* fibonacci(): IterableIterator { itiriri(fibonacci()) .take(10) - .forEach(fib => console.log(fib)); + .forEach((fib) => console.log(fib)); ``` **[⬆ back to top](#table-of-contents)** @@ -1179,7 +1241,7 @@ Using getters and setters to access data from objects that encapsulate behavior type BankAccount = { balance: number; // ... -} +}; const value = 100; const account: BankAccount = { @@ -1227,14 +1289,14 @@ account.balance = 100; ### Make objects have private/protected members -TypeScript supports `public` *(default)*, `protected` and `private` accessors on class members. +TypeScript supports `public` _(default)_, `protected` and `private` accessors on class members. **Bad:** ```ts class Circle { radius: number; - + constructor(radius: number) { this.radius = radius; } @@ -1253,8 +1315,7 @@ class Circle { ```ts class Circle { - constructor(private readonly radius: number) { - } + constructor(private readonly radius: number) {} perimeter() { return 2 * Math.PI * this.radius; @@ -1270,7 +1331,7 @@ class Circle { ### Prefer immutability -TypeScript's type system allows you to mark individual properties on an interface/class as *readonly*. This allows you to work in a functional way (an unexpected mutation is bad). +TypeScript's type system allows you to mark individual properties on an interface/class as _readonly_. This allows you to work in a functional way (an unexpected mutation is bad). For more advanced scenarios there is a built-in type `Readonly` that takes a type `T` and marks all of its properties as readonly using mapped types (see [mapped types](https://www.typescriptlang.org/docs/handbook/advanced-types.html#mapped-types)). **Bad:** @@ -1299,7 +1360,7 @@ It doesn't allow changes such as `push()` and `fill()`, but can use features suc **Bad:** ```ts -const array: number[] = [ 1, 3, 5 ]; +const array: number[] = [1, 3, 5]; array = []; // error array.push(100); // array will be updated ``` @@ -1307,7 +1368,7 @@ array.push(100); // array will be updated **Good:** ```ts -const array: ReadonlyArray = [ 1, 3, 5 ]; +const array: ReadonlyArray = [1, 3, 5]; array = []; // error array.push(100); // error ``` @@ -1326,11 +1387,11 @@ Prefer [const assertions](https://github.com/microsoft/TypeScript/wiki/What's-ne ```ts const config = { - hello: 'world' + hello: 'world', }; config.hello = 'world'; // value is changed -const array = [ 1, 3, 5 ]; +const array = [1, 3, 5]; array[0] = 10; // value is changed // writable objects is returned @@ -1347,12 +1408,12 @@ result.value = 200; // value is changed ```ts // read-only object const config = { - hello: 'world' + hello: 'world', } as const; config.hello = 'world'; // error // read-only array -const array = [ 1, 3, 5 ] as const; +const array = [1, 3, 5] as const; array[0] = 10; // error // You can return read-only objects @@ -1390,22 +1451,21 @@ interface Config { type Shape = { // ... -} +}; ``` **Good:** ```ts - type EmailConfig = { // ... -} +}; type DbConfig = { // ... -} +}; -type Config = EmailConfig | DbConfig; +type Config = EmailConfig | DbConfig; // ... @@ -1428,38 +1488,71 @@ class Square implements Shape { ### Classes should be small -The class' size is measured by its responsibility. Following the *Single Responsibility principle* a class should be small. +The class' size is measured by its responsibility. Following the _Single Responsibility principle_ a class should be small. **Bad:** ```ts class Dashboard { - getLanguage(): string { /* ... */ } - setLanguage(language: string): void { /* ... */ } - showProgress(): void { /* ... */ } - hideProgress(): void { /* ... */ } - isDirty(): boolean { /* ... */ } - disable(): void { /* ... */ } - enable(): void { /* ... */ } - addSubscription(subscription: Subscription): void { /* ... */ } - removeSubscription(subscription: Subscription): void { /* ... */ } - addUser(user: User): void { /* ... */ } - removeUser(user: User): void { /* ... */ } - goToHomePage(): void { /* ... */ } - updateProfile(details: UserDetails): void { /* ... */ } - getVersion(): string { /* ... */ } + getLanguage(): string { + /* ... */ + } + setLanguage(language: string): void { + /* ... */ + } + showProgress(): void { + /* ... */ + } + hideProgress(): void { + /* ... */ + } + isDirty(): boolean { + /* ... */ + } + disable(): void { + /* ... */ + } + enable(): void { + /* ... */ + } + addSubscription(subscription: Subscription): void { + /* ... */ + } + removeSubscription(subscription: Subscription): void { + /* ... */ + } + addUser(user: User): void { + /* ... */ + } + removeUser(user: User): void { + /* ... */ + } + goToHomePage(): void { + /* ... */ + } + updateProfile(details: UserDetails): void { + /* ... */ + } + getVersion(): string { + /* ... */ + } // ... } - ``` **Good:** ```ts class Dashboard { - disable(): void { /* ... */ } - enable(): void { /* ... */ } - getVersion(): string { /* ... */ } + disable(): void { + /* ... */ + } + enable(): void { + /* ... */ + } + getVersion(): string { + /* ... */ + } } // split the responsibilities by moving the remaining methods to other classes @@ -1471,10 +1564,10 @@ class Dashboard { ### High cohesion and low coupling Cohesion defines the degree to which class members are related to each other. Ideally, all fields within a class should be used by each method. -We then say that the class is *maximally cohesive*. In practice, this, however, is not always possible, nor even advisable. You should however prefer cohesion to be high. +We then say that the class is _maximally cohesive_. In practice, this, however, is not always possible, nor even advisable. You should however prefer cohesion to be high. + +Coupling refers to how related or dependent are two classes toward each other. Classes are said to be low coupled if changes in one of them don't affect the other one. -Coupling refers to how related or dependent are two classes toward each other. Classes are said to be low coupled if changes in one of them don't affect the other one. - Good software design has **high cohesion** and **low coupling**. **Bad:** @@ -1485,10 +1578,7 @@ class UserManager { // It makes clear evidence that the class is holding more than a single responsibility. // If I need only to create the service to get the transactions for a user, // I'm still forced to pass and instance of `emailSender`. - constructor( - private readonly db: Database, - private readonly emailSender: EmailSender) { - } + constructor(private readonly db: Database, private readonly emailSender: EmailSender) {} async getUser(id: number): Promise { return await db.users.findOne({ id }); @@ -1516,8 +1606,7 @@ class UserManager { ```ts class UserService { - constructor(private readonly db: Database) { - } + constructor(private readonly db: Database) {} async getUser(id: number): Promise { return await this.db.users.findOne({ id }); @@ -1529,8 +1618,7 @@ class UserService { } class UserNotifier { - constructor(private readonly emailSender: EmailSender) { - } + constructor(private readonly emailSender: EmailSender) {} async sendGreeting(): Promise { await this.emailSender.send('Welcome!'); @@ -1550,8 +1638,8 @@ class UserNotifier { ### Prefer composition over inheritance -As stated famously in [Design Patterns](https://en.wikipedia.org/wiki/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. - +As stated famously in [Design Patterns](https://en.wikipedia.org/wiki/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: 1. Your inheritance represents an "is-a" relationship and not a "has-a" relationship (Human->Animal vs. User->UserDetails). @@ -1564,21 +1652,14 @@ You might be wondering then, "when should I use inheritance?" It depends on your ```ts class Employee { - constructor( - private readonly name: string, - private readonly email: string) { - } + constructor(private readonly name: string, private readonly email: string) {} // ... } // Bad because Employees "have" tax data. EmployeeTaxData is not a type of Employee class EmployeeTaxData extends Employee { - constructor( - name: string, - email: string, - private readonly ssn: string, - private readonly salary: number) { + constructor(name: string, email: string, private readonly ssn: string, private readonly salary: number) { super(name, email); } @@ -1592,10 +1673,7 @@ class EmployeeTaxData extends Employee { class Employee { private taxData: EmployeeTaxData; - constructor( - private readonly name: string, - private readonly email: string) { - } + constructor(private readonly name: string, private readonly email: string) {} setTaxData(ssn: string, salary: number): Employee { this.taxData = new EmployeeTaxData(ssn, salary); @@ -1606,10 +1684,7 @@ class Employee { } class EmployeeTaxData { - constructor( - public readonly ssn: string, - public readonly salary: number) { - } + constructor(public readonly ssn: string, public readonly salary: number) {} // ... } @@ -1690,11 +1765,7 @@ class QueryBuilder { // ... -const query = new QueryBuilder() - .from('users') - .page(1, 100) - .orderBy('firstName', 'lastName') - .build(); +const query = new QueryBuilder().from('users').page(1, 100).orderBy('firstName', 'lastName').build(); ``` **[⬆ back to top](#table-of-contents)** @@ -1709,8 +1780,7 @@ As stated in Clean Code, "There should never be more than one reason for a class ```ts class UserSettings { - constructor(private readonly user: User) { - } + constructor(private readonly user: User) {} changeSettings(settings: UserSettings) { if (this.verifyCredentials()) { @@ -1728,15 +1798,13 @@ class UserSettings { ```ts class UserAuth { - constructor(private readonly user: User) { - } + constructor(private readonly user: User) {} verifyCredentials() { // ... } } - class UserSettings { private readonly auth: UserAuth; @@ -1778,8 +1846,7 @@ class NodeAdapter extends Adapter { } class HttpRequester { - constructor(private readonly adapter: Adapter) { - } + constructor(private readonly adapter: Adapter) {} async fetch(url: string): Promise { if (this.adapter instanceof AjaxAdapter) { @@ -1815,7 +1882,7 @@ class AjaxAdapter extends Adapter { super(); } - async request(url: string): Promise{ + async request(url: string): Promise { // request and return promise } @@ -1827,7 +1894,7 @@ class NodeAdapter extends Adapter { super(); } - async request(url: string): Promise{ + async request(url: string): Promise { // request and return promise } @@ -1835,8 +1902,7 @@ class NodeAdapter extends Adapter { } class HttpRequester { - constructor(private readonly adapter: Adapter) { - } + constructor(private readonly adapter: Adapter) {} async fetch(url: string): Promise { const response = await this.adapter.request(url); @@ -1849,19 +1915,15 @@ class HttpRequester { ### Liskov Substitution Principle (LSP) -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. - +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 parent 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. **Bad:** ```ts class Rectangle { - constructor( - protected width: number = 0, - protected height: number = 0) { - - } + constructor(protected width: number = 0, protected height: number = 0) {} setColor(color: string): this { // ... @@ -1902,10 +1964,7 @@ class Square extends Rectangle { function renderLargeRectangles(rectangles: Rectangle[]) { rectangles.forEach((rectangle) => { - const area = rectangle - .setWidth(4) - .setHeight(5) - .getArea(); // BAD: Returns 25 for Square. Should be 20. + const area = rectangle.setWidth(4).setHeight(5).getArea(); // BAD: Returns 25 for Square. Should be 20. rectangle.render(area); }); } @@ -1930,9 +1989,7 @@ abstract class Shape { } class Rectangle extends Shape { - constructor( - private readonly width = 0, - private readonly height = 0) { + constructor(private readonly width = 0, private readonly height = 0) { super(); } @@ -1981,8 +2038,8 @@ interface SmartPrinter { class AllInOnePrinter implements SmartPrinter { print() { // ... - } - + } + fax() { // ... } @@ -1995,8 +2052,8 @@ class AllInOnePrinter implements SmartPrinter { class EconomicPrinter implements SmartPrinter { print() { // ... - } - + } + fax() { throw new Error('Fax not supported.'); } @@ -2025,8 +2082,8 @@ interface Scanner { class AllInOnePrinter implements Printer, Fax, Scanner { print() { // ... - } - + } + fax() { // ... } @@ -2053,8 +2110,8 @@ This principle states two essential things: 2. 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 Angular, 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 bad development pattern because it makes your code hard to refactor. - +This can be hard to understand at first, but if you've worked with Angular, 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 bad development pattern because it makes your code hard to refactor. + DIP is usually achieved by a using an inversion of control (IoC) container. An example of a powerful IoC container for TypeScript is [InversifyJs](https://www.npmjs.com/package/inversify) **Bad:** @@ -2067,7 +2124,7 @@ const readFile = promisify(readFileCb); type ReportData = { // .. -} +}; class XmlFormatter { parse(content: string): T { @@ -2076,7 +2133,6 @@ class XmlFormatter { } class ReportReader { - // BAD: We have created a dependency on a specific request implementation. // We should just have ReportReader depend on a parse method: `parse` private readonly formatter = new XmlFormatter(); @@ -2102,7 +2158,7 @@ const readFile = promisify(readFileCb); type ReportData = { // .. -} +}; interface Formatter { parse(content: string): T; @@ -2114,7 +2170,6 @@ class XmlFormatter implements Formatter { } } - class JsonFormatter implements Formatter { parse(content: string): T { // Converts a JSON string to an object T @@ -2122,8 +2177,7 @@ class JsonFormatter implements Formatter { } class ReportReader { - constructor(private readonly formatter: Formatter) { - } + constructor(private readonly formatter: Formatter) {} async read(path: string): Promise { const text = await readFile(path, 'UTF8'); @@ -2148,7 +2202,7 @@ Testing is more important than shipping. If you have no tests or an inadequate a 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 testing framework, you also need to use a good [coverage tool](https://github.com/gotwarlost/istanbul). -There's no excuse to not write tests. There are [plenty of good JS test frameworks](http://jstherightway.org/#testing-tools) with typings support for TypeScript, 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. +There's no excuse to not write tests. There are [plenty of good JS test frameworks](http://jstherightway.org/#testing-tools) with typings support for TypeScript, 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. ### The three laws of TDD @@ -2170,7 +2224,7 @@ Clean tests should follow the rules: - **Repeatable** tests should be repeatable in any environment and there should be no excuse for why they fail. -- **Self-Validating** a test should answer with either *Passed* or *Failed*. You don't need to compare log files to answer if a test passed. +- **Self-Validating** a test should answer with either _Passed_ or _Failed_. You don't need to compare log files to answer if a test passed. - **Timely** unit tests should be written before the production code. If you write tests after the production code, you might find writing tests too hard. @@ -2178,7 +2232,7 @@ Clean tests should follow the rules: ### Single concept per test -Tests should also follow the *Single Responsibility Principle*. Make only one assert per unit test. +Tests should also follow the _Single Responsibility Principle_. Make only one assert per unit test. **Bad:** @@ -2264,7 +2318,7 @@ describe('Calendar', () => { ### Prefer promises vs callbacks -Callbacks aren't clean, and they cause excessive amounts of nesting *(the callback hell)*. +Callbacks aren't clean, and they cause excessive amounts of nesting _(the callback hell)_. There are utilities that transform existing functions using the callback style to a version that returns promises (for Node.js see [`util.promisify`](https://nodejs.org/dist/latest-v8.x/docs/api/util.html#util_util_promisify_original), for general purpose see [pify](https://www.npmjs.com/package/pify), [es6-promisify](https://www.npmjs.com/package/es6-promisify)) @@ -2309,23 +2363,22 @@ import { promisify } from 'util'; const write = promisify(writeFile); function downloadPage(url: string, saveTo: string): Promise { - return get(url) - .then(response => write(saveTo, response)); + return get(url).then((response) => write(saveTo, response)); } downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html') - .then(content => console.log(content)) - .catch(error => console.error(error)); + .then((content) => console.log(content)) + .catch((error) => console.error(error)); ``` -Promises supports a few helper methods that help make code more concise: +Promises supports a few helper methods that help make code more concise: -| Pattern | Description | -| ------------------------ | ----------------------------------------- | -| `Promise.resolve(value)` | Convert a value into a resolved promise. | -| `Promise.reject(error)` | Convert an error into a rejected promise. | +| Pattern | Description | +| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `Promise.resolve(value)` | Convert a value into a resolved promise. | +| `Promise.reject(error)` | Convert an error into a rejected promise. | | `Promise.all(promises)` | Returns a new promise which is fulfilled with an array of fulfillment values for the passed promises or rejects with the reason of the first promise that rejects. | -| `Promise.race(promises)`| Returns a new promise which is fulfilled/rejected with the result/error of the first settled promise from the array of passed promises. | +| `Promise.race(promises)` | Returns a new promise which is fulfilled/rejected with the result/error of the first settled promise from the array of passed promises. | `Promise.all` is especially useful when there is a need to run tasks in parallel. `Promise.race` makes it easier to implement things like timeouts for promises. @@ -2345,12 +2398,12 @@ import { promisify } from 'util'; const write = util.promisify(writeFile); function downloadPage(url: string, saveTo: string): Promise { - return get(url).then(response => write(saveTo, response)); + return get(url).then((response) => write(saveTo, response)); } downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html') - .then(content => console.log(content)) - .catch(error => console.error(error)); + .then((content) => console.log(content)) + .catch((error) => console.error(error)); ``` **Good:** @@ -2428,8 +2481,8 @@ There are also other alternatives, not to use the `throw` syntax and instead alw Consider the following example: ```ts -type Result = { isError: false, value: R }; -type Failure = { isError: true, error: E }; +type Result = { isError: false; value: R }; +type Failure = { isError: true; error: E }; type Failable = Result | Failure; function calculateTotal(items: Item[]): Failable { @@ -2471,7 +2524,7 @@ try { **Good:** ```ts -import { logger } from './logging' +import { logger } from './logging'; try { functionThatMightThrow(); @@ -2501,7 +2554,7 @@ getUser() **Good:** ```ts -import { logger } from './logging' +import { logger } from './logging'; getUser() .then((user: User) => { @@ -2525,7 +2578,7 @@ try { ## Formatting -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. The general rule to follow is *keep consistent formatting rules*. +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. The general rule to follow is _keep consistent formatting rules_. For TypeScript there is a powerful tool called [ESLint](https://typescript-eslint.io/). It's a static analysis tool that can help you improve dramatically the readability and maintainability of your code. There are ready to use ESLint configurations that you can reference in your projects: @@ -2543,7 +2596,7 @@ If you are looking for help in migrating from TSLint to ESLint, you can check ou ### 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*. +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_. **Bad:** @@ -2557,8 +2610,12 @@ const Artists = ['ACDC', 'Led Zeppelin', 'The Beatles']; function eraseDatabase() {} function restore_database() {} -type animal = { /* ... */ } -type Container = { /* ... */ } +type animal = { + /* ... */ +}; +type Container = { + /* ... */ +}; ``` **Good:** @@ -2576,8 +2633,12 @@ const beatlesSongs = SONGS.filter((song) => isBeatlesSong(song)); function eraseDatabase() {} function restoreDatabase() {} -type Animal = { /* ... */ } -type Container = { /* ... */ } +type Animal = { + /* ... */ +}; +type Container = { + /* ... */ +}; ``` Prefer using `PascalCase` for class, interface, type and namespace names. @@ -2595,8 +2656,7 @@ We tend to read code from top-to-bottom, like a newspaper. Because of this, make ```ts class PerformanceReview { - constructor(private readonly employee: Employee) { - } + constructor(private readonly employee: Employee) {} private lookupPeers() { return db.lookup(this.employee.id, 'peers'); @@ -2636,8 +2696,7 @@ review.review(); ```ts class PerformanceReview { - constructor(private readonly employee: Employee) { - } + constructor(private readonly employee: Employee) {} review() { this.getPeerReviews(); @@ -2681,6 +2740,7 @@ With clean and easy to read import statements you can quickly see the dependenci - Import statements should be alphabetized and grouped. - Unused imports should be removed. +- Imports should be named imports to a specific resource - Named imports must be alphabetized (i.e. `import {A, B, C} from 'foo';`) - Import sources must be alphabetized within groups, i.e.: `import * as foo from 'a'; import * as bar from 'b';` - Prefer using `import type` instead of `import` when importing only types from a file to avoid dependency cycles, as these imports are erased at runtime @@ -2695,6 +2755,28 @@ With clean and easy to read import statements you can quickly see the dependenci **Bad:** +```ts +import { services } from '../services'; +import { errors } from '../errors'; +import { types } from '../types'; + +const apiService = services.api; +const badError = errors.bad; +const interfaceType = types.interface; +``` + +**Good:** + +```ts +import { api } from '../services/api'; +import { bad } from '../errors/bad'; +import { interface } from '../types/interface'; + +// normal usage +``` + +**Bad:** + ```ts import { TypeDefinition } from '../types/typeDefinition'; import { AttributeTypes } from '../model/attribute'; @@ -2761,26 +2843,29 @@ import { UserService } from '@services/UserService'; ## Comments The use of a comments is an indication of failure to express without them. Code should be the only source of truth. - + > Don’t comment bad code—rewrite it. -> — *Brian W. Kernighan and P. J. Plaugher* +> — _Brian W. Kernighan and P. J. Plaugher_ ### Prefer self explanatory code instead of comments -Comments are an apology, not a requirement. Good code *mostly* documents itself. +Comments are an apology, not a requirement. Good code _mostly_ documents itself. **Bad:** ```ts // Check if subscription is active. -if (subscription.endDate > Date.now) { } +if (subscription.endDate > Date.now) { +} ``` **Good:** ```ts const isSubscriptionActive = subscription.endDate > Date.now; -if (isSubscriptionActive) { /* ... */ } +if (isSubscriptionActive) { + /* ... */ +} ``` **[⬆ back to top](#table-of-contents)** @@ -2797,7 +2882,7 @@ type User = { email: string; // age: number; // jobPosition: string; -} +}; ``` **Good:** @@ -2806,7 +2891,7 @@ type User = { type User = { name: string; email: string; -} +}; ``` **[⬆ back to top](#table-of-contents)** @@ -2873,7 +2958,7 @@ class Client { private describeContact(): string { // ... } -}; +} ``` **Good:** @@ -2896,7 +2981,7 @@ class Client { private describeContact(): string { // ... } -}; +} ``` **[⬆ back to top](#table-of-contents)** @@ -2905,9 +2990,9 @@ class Client { When you find yourself that you need to leave notes in the code for some later improvements, do that using `// TODO` comments. Most IDE have special support for those kinds of comments so that -you can quickly go over the entire list of todos. +you can quickly go over the entire list of todos. -Keep in mind however that a *TODO* comment is not an excuse for bad code. +Keep in mind however that a _TODO_ comment is not an excuse for bad code. **Bad:** @@ -2932,8 +3017,9 @@ function getActiveSubscriptions(): Promise { ## Translations This is also available in other languages: + - ![br](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Brazil.png) **Brazilian Portuguese**: [vitorfreitas/clean-code-typescript](https://github.com/vitorfreitas/clean-code-typescript) -- ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Chinese**: +- ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Chinese**: - [beginor/clean-code-typescript](https://github.com/beginor/clean-code-typescript) - [pipiliang/clean-code-typescript](https://github.com/pipiliang/clean-code-typescript) - ![fr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/France.png) **French**: [ralflorent/clean-code-typescript](https://github.com/ralflorent/clean-code-typescript) @@ -2947,6 +3033,6 @@ This is also available in other languages: References will be added once translations are completed. Check this [discussion](https://github.com/labs42io/clean-code-typescript/issues/15) for more details and progress. -You can make an indispensable contribution to *Clean Code* community by translating this to your language. +You can make an indispensable contribution to _Clean Code_ community by translating this to your language. **[⬆ back to top](#table-of-contents)** diff --git a/examples/.gitkeep b/examples/.gitkeep new file mode 100644 index 0000000..e69de29