Skip to content

Commit 6a37142

Browse files
Minor improvements regarding types (#55)
* Fix typo in 'Remove Duplicate Code' section and added union type suggestion * Make paragraph about `ReadonlyArray<T>` read more fluently * Remove side effects from function in async/await example While this is not the point of the example, it would be good to keep the code consistent across examples * Elaborate capitalization common best practices * Add mention of `import type` to import section
1 parent 263c7a5 commit 6a37142

File tree

1 file changed

+33
-7
lines changed

1 file changed

+33
-7
lines changed

README.md

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ class Manager {
563563
}
564564
}
565565

566-
function showEmployeeList(employee: Developer | Manager) {
566+
function showEmployeeList(employee: (Developer | Manager)[]) {
567567
employee.forEach((employee) => {
568568
const expectedSalary = employee.calculateExpectedSalary();
569569
const experience = employee.getExperience();
@@ -580,6 +580,25 @@ function showEmployeeList(employee: Developer | Manager) {
580580
}
581581
```
582582

583+
You may also consider adding a union type, or common parent class if it suits your abstraction.
584+
```ts
585+
class Developer {
586+
// ...
587+
}
588+
589+
class Manager {
590+
// ...
591+
}
592+
593+
type Employee = Developer | Manager
594+
595+
function showEmployeeList(employee: Employee[]) {
596+
// ...
597+
});
598+
}
599+
600+
```
601+
583602
You should be critical about code duplication. Sometimes there is a tradeoff between duplicated code and increased complexity by introducing unnecessary abstraction. When two implementations from two different modules look similar but live in different domains, duplication might be acceptable and preferred over extracting the common code. The extracted common code, in this case, introduces an indirect dependency between the two modules.
584603

585604
**[⬆ back to top](#table-of-contents)**
@@ -1274,15 +1293,15 @@ interface Config {
12741293
}
12751294
```
12761295

1277-
Case of Array, you can create a read-only array by using `ReadonlyArray<T>`.
1278-
do not allow changes such as `push()` and `fill()`, but can use features such as `concat()` and `slice()` that do not change the value.
1296+
For arrays, you can create a read-only array by using `ReadonlyArray<T>`.
1297+
It doesn't allow changes such as `push()` and `fill()`, but can use features such as `concat()` and `slice()` that do not change the array's value.
12791298

12801299
**Bad:**
12811300

12821301
```ts
12831302
const array: number[] = [ 1, 3, 5 ];
12841303
array = []; // error
1285-
array.push(100); // array will updated
1304+
array.push(100); // array will be updated
12861305
```
12871306

12881307
**Good:**
@@ -2343,15 +2362,15 @@ import { promisify } from 'util';
23432362

23442363
const write = promisify(writeFile);
23452364

2346-
async function downloadPage(url: string, saveTo: string): Promise<string> {
2365+
async function downloadPage(url: string): Promise<string> {
23472366
const response = await get(url);
2348-
await write(saveTo, response);
23492367
return response;
23502368
}
23512369

23522370
// somewhere in an async function
23532371
try {
2354-
const content = await downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html');
2372+
const content = await downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin');
2373+
await write('article.html', content);
23552374
console.log(content);
23562375
} catch (error) {
23572376
console.error(error);
@@ -2551,6 +2570,9 @@ const DAYS_IN_MONTH = 30;
25512570
const SONGS = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
25522571
const ARTISTS = ['ACDC', 'Led Zeppelin', 'The Beatles'];
25532572

2573+
const discography = getArtistDiscography('ACDC');
2574+
const beatlesSongs = SONGS.filter((song) => isBeatlesSong(song));
2575+
25542576
function eraseDatabase() {}
25552577
function restoreDatabase() {}
25562578

@@ -2560,6 +2582,7 @@ type Container = { /* ... */ }
25602582
25612583
Prefer using `PascalCase` for class, interface, type and namespace names.
25622584
Prefer using `camelCase` for variables, functions and class members.
2585+
Prefer using capitalized `SNAKE_CASE` for constants.
25632586
25642587
**[⬆ back to top](#table-of-contents)**
25652588
@@ -2660,6 +2683,7 @@ With clean and easy to read import statements you can quickly see the dependenci
26602683
- Unused imports should be removed.
26612684
- Named imports must be alphabetized (i.e. `import {A, B, C} from 'foo';`)
26622685
- Import sources must be alphabetized within groups, i.e.: `import * as foo from 'a'; import * as bar from 'b';`
2686+
- 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
26632687
- Groups of imports are delineated by blank lines.
26642688
- Groups must respect following order:
26652689
- Polyfills (i.e. `import 'reflect-metadata';`)
@@ -2674,6 +2698,7 @@ With clean and easy to read import statements you can quickly see the dependenci
26742698
```ts
26752699
import { TypeDefinition } from '../types/typeDefinition';
26762700
import { AttributeTypes } from '../model/attribute';
2701+
import { Customer, Credentials } from '../model/types';
26772702
import { ApiCredentials, Adapters } from './common/api/authorization';
26782703
import fs from 'fs';
26792704
import { ConfigPlugin } from './plugins/config/configPlugin';
@@ -2691,6 +2716,7 @@ import { BindingScopeEnum, Container } from 'inversify';
26912716

26922717
import { AttributeTypes } from '../model/attribute';
26932718
import { TypeDefinition } from '../types/typeDefinition';
2719+
import type { Customer, Credentials } from '../model/types';
26942720

26952721
import { ApiCredentials, Adapters } from './common/api/authorization';
26962722
import { ConfigPlugin } from './plugins/config/configPlugin';

0 commit comments

Comments
 (0)