mirror of
https://github.com/labs42io/clean-code-typescript.git
synced 2024-09-19 22:34:04 +00:00
SOLID
This commit is contained in:
Dumitru Deveatii
parent
519debc73b
commit
1e20f82875
445
README.md
445
README.md
|
|
@ -10,7 +10,7 @@ Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-cod
|
|||
3. [Functions](#functions)
|
||||
4. [Objects and Data Structures](#objects-and-data-structures)
|
||||
5. [Classes](#classes) *TODO*
|
||||
6. [SOLID](#solid) *TODO*
|
||||
6. [SOLID](#solid)
|
||||
7. [Testing](#testing)
|
||||
8. [Concurrency](#concurrency)
|
||||
9. [Error Handling](#error-handling)
|
||||
|
|
@ -1095,15 +1095,454 @@ interface Config {
|
|||
}
|
||||
```
|
||||
|
||||
## SOLID
|
||||
|
||||
### Single Responsibility Principle (SRP)
|
||||
|
||||
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.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```ts
|
||||
class UserSettings {
|
||||
constructor(private readonly user: User) {
|
||||
}
|
||||
|
||||
changeSettings(settings: UserSettings) {
|
||||
if (this.verifyCredentials()) {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
|
||||
verifyCredentials() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```ts
|
||||
class UserAuth {
|
||||
constructor(private readonly user: User) {
|
||||
}
|
||||
|
||||
verifyCredentials() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
class UserSettings {
|
||||
private readonly auth: UserAuth;
|
||||
|
||||
constructor(private readonly user: User) {
|
||||
this.auth = new UserAuth(user);
|
||||
}
|
||||
|
||||
changeSettings(settings: UserSettings) {
|
||||
if (this.auth.verifyCredentials()) {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Open/Closed Principle (OCP)
|
||||
|
||||
As stated by Bertrand Meyer, "software entities (classes, modules, functions, 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.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```ts
|
||||
class AjaxAdapter extends Adapter {
|
||||
constructor() {
|
||||
super();
|
||||
}
|
||||
|
||||
// ...
|
||||
}
|
||||
|
||||
class NodeAdapter extends Adapter {
|
||||
constructor() {
|
||||
super();
|
||||
}
|
||||
|
||||
// ...
|
||||
}
|
||||
|
||||
class HttpRequester {
|
||||
constructor(private readonly adapter: Adapter) {
|
||||
}
|
||||
|
||||
async fetch<T>(url: string): Promise<T> {
|
||||
if (this.adapter instanceof AjaxAdapter) {
|
||||
const response = await makeAjaxCall<T>(url);
|
||||
// transform response and return
|
||||
} else if (this.adapter instanceof NodeAdapter) {
|
||||
const response = await makeHttpCall<T>(url);
|
||||
// transform response and return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function makeAjaxCall<T>(url: string): Promise<T> {
|
||||
// request and return promise
|
||||
}
|
||||
|
||||
function makeHttpCall<T>(url: string): Promise<T> {
|
||||
// request and return promise
|
||||
}
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```ts
|
||||
abstract class Adapter {
|
||||
abstract async request<T>(url: string): Promise<T>;
|
||||
}
|
||||
|
||||
class AjaxAdapter extends Adapter {
|
||||
constructor() {
|
||||
super();
|
||||
}
|
||||
|
||||
async request<T>(url: string): Promise<T>{
|
||||
// request and return promise
|
||||
}
|
||||
|
||||
// ...
|
||||
}
|
||||
|
||||
class NodeAdapter extends Adapter {
|
||||
constructor() {
|
||||
super();
|
||||
}
|
||||
|
||||
async request<T>(url: string): Promise<T>{
|
||||
// request and return promise
|
||||
}
|
||||
|
||||
// ...
|
||||
}
|
||||
|
||||
class HttpRequester {
|
||||
constructor(private readonly adapter: Adapter) {
|
||||
}
|
||||
|
||||
async fetch<T>(url: string): Promise<T> {
|
||||
const response = await this.adapter.request<T>(url);
|
||||
// transform response and return
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### 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.
|
||||
|
||||
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.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```ts
|
||||
class Rectangle {
|
||||
constructor(
|
||||
protected width: number = 0,
|
||||
protected height: number = 0) {
|
||||
|
||||
}
|
||||
|
||||
setColor(color: string) {
|
||||
// ...
|
||||
}
|
||||
|
||||
render(area: number) {
|
||||
// ...
|
||||
}
|
||||
|
||||
setWidth(width: number) {
|
||||
this.width = width;
|
||||
}
|
||||
|
||||
setHeight(height: number) {
|
||||
this.height = height;
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this.width * this.height;
|
||||
}
|
||||
}
|
||||
|
||||
class Square extends Rectangle {
|
||||
setWidth(width: number) {
|
||||
this.width = width;
|
||||
this.height = width;
|
||||
}
|
||||
|
||||
setHeight(height: number) {
|
||||
this.width = height;
|
||||
this.height = height;
|
||||
}
|
||||
}
|
||||
|
||||
function renderLargeRectangles(rectangles: Rectangle[]) {
|
||||
rectangles.forEach((rectangle) => {
|
||||
rectangle.setWidth(4);
|
||||
rectangle.setHeight(5);
|
||||
const area = rectangle.getArea(); // BAD: Returns 25 for Square. Should be 20.
|
||||
rectangle.render(area);
|
||||
});
|
||||
}
|
||||
|
||||
const rectangles = [new Rectangle(), new Rectangle(), new Square()];
|
||||
renderLargeRectangles(rectangles);
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```ts
|
||||
abstract class Shape {
|
||||
setColor(color: string) {
|
||||
// ...
|
||||
}
|
||||
|
||||
render(area: number) {
|
||||
// ...
|
||||
}
|
||||
|
||||
abstract getArea(): number;
|
||||
}
|
||||
|
||||
class Rectangle extends Shape {
|
||||
constructor(
|
||||
private readonly width = 0,
|
||||
private readonly height = 0) {
|
||||
super();
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this.width * this.height;
|
||||
}
|
||||
}
|
||||
|
||||
class Square extends Shape {
|
||||
constructor(private readonly length: number) {
|
||||
super();
|
||||
}
|
||||
|
||||
getArea(): number {
|
||||
return this.length * this.length;
|
||||
}
|
||||
}
|
||||
|
||||
function renderLargeShapes(shapes: Shape[]) {
|
||||
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);
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Interface Segregation Principle (ISP)
|
||||
|
||||
ISP states that "Clients should not be forced to depend upon interfaces that they do not use.". This principle is very much related to the Single Responsibility Principle.
|
||||
What it really means is that you should always design your abstractions in a way that the clients that are using the exposed methods do not get the whole pie instead. That also include imposing the clients with the burden of implementing methods that they don’t actually need.
|
||||
|
||||
**Bad:**
|
||||
|
||||
```ts
|
||||
interface ISmartPrinter {
|
||||
print();
|
||||
|
||||
fax();
|
||||
|
||||
scan();
|
||||
}
|
||||
|
||||
class AllInOnePrinter implements ISmartPrinter {
|
||||
print() {
|
||||
// ...
|
||||
}
|
||||
|
||||
fax() {
|
||||
// ...
|
||||
}
|
||||
|
||||
scan() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
|
||||
class EconomicPrinter implements ISmartPrinter {
|
||||
print() {
|
||||
// ...
|
||||
}
|
||||
|
||||
fax() {
|
||||
throw new Error('Fax not supported.');
|
||||
}
|
||||
|
||||
scan() {
|
||||
throw new Error('Scan not supported.');
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```ts
|
||||
interface IPrinter {
|
||||
print();
|
||||
}
|
||||
|
||||
interface IFax {
|
||||
fax();
|
||||
}
|
||||
|
||||
interface IScanner {
|
||||
scan();
|
||||
}
|
||||
|
||||
class AllInOnePrinter implements IPrinter, IFax, IScanner {
|
||||
print() {
|
||||
// ...
|
||||
}
|
||||
|
||||
fax() {
|
||||
// ...
|
||||
}
|
||||
|
||||
scan() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
|
||||
class EconomicPrinter implements IPrinter {
|
||||
print() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
### Dependency Inversion Principle (DIP)
|
||||
|
||||
This principle states two essential things:
|
||||
|
||||
1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
|
||||
|
||||
2. Abstractions should not depend upon details. Details should depend on abstractions.
|
||||
|
||||
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:**
|
||||
|
||||
```ts
|
||||
import { readFile as readFileCb } from 'fs';
|
||||
import { promisify } from 'util';
|
||||
|
||||
const readFile = promisify(readFileCb);
|
||||
|
||||
type ReportData = {
|
||||
// ..
|
||||
}
|
||||
|
||||
class XmlFormatter {
|
||||
parse<T>(content: string): T {
|
||||
// Converts an XML string to an object T
|
||||
}
|
||||
}
|
||||
|
||||
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();
|
||||
|
||||
async read(path: string): Promise<ReportData> {
|
||||
const text = await readFile(path, 'UTF8');
|
||||
return this.formatter.parse<ReportData>(text);
|
||||
}
|
||||
}
|
||||
|
||||
// ...
|
||||
const reader = new ReportReader();
|
||||
await report = await reader.read('report.xml');
|
||||
```
|
||||
|
||||
**Good:**
|
||||
|
||||
```ts
|
||||
import { readFile as readFileCb } from 'fs';
|
||||
import { promisify } from 'util';
|
||||
|
||||
const readFile = promisify(readFileCb);
|
||||
|
||||
type ReportData = {
|
||||
// ..
|
||||
}
|
||||
|
||||
interface Formatter {
|
||||
parse<T>(content: string): T;
|
||||
}
|
||||
|
||||
class XmlFormatter implements Formatter {
|
||||
parse<T>(content: string): T {
|
||||
// Converts an XML string to an object T
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
class JsonFormatter implements Formatter {
|
||||
parse<T>(content: string): T {
|
||||
// Converts a JSON string to an object T
|
||||
}
|
||||
}
|
||||
|
||||
class ReportReader {
|
||||
constructor(private readonly formatter: Formatter){
|
||||
|
||||
}
|
||||
|
||||
async read(path: string): Promise<ReportData> {
|
||||
const text = await readFile(path, 'UTF8');
|
||||
return this.formatter.parse<ReportData>(text);
|
||||
}
|
||||
}
|
||||
|
||||
// ...
|
||||
const reader = new ReportReader(new XmlFormatter());
|
||||
await report = await reader.read('report.xml');
|
||||
|
||||
// or if we had to read a json report:
|
||||
const reader = new ReportReader(new JsonFormatter());
|
||||
await report = await reader.read('report.json');
|
||||
```
|
||||
|
||||
**[⬆ back to top](#table-of-contents)**
|
||||
|
||||
## Testing
|
||||
|
||||
Testing 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 testing 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 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 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 TDDD
|
||||
### The three laws of TDD
|
||||
|
||||
1. You are not allowed to write any production code unless it is to make a failing unit test pass.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue