Skip to content

Commit 60b4517

Browse files
authored
Merge pull request #1 from ryanmcdermott/master
Update
2 parents f350f2b + 383dd49 commit 60b4517

File tree

1 file changed

+58
-76
lines changed

1 file changed

+58
-76
lines changed

README.md

Lines changed: 58 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -79,35 +79,38 @@ can help identify unnamed constants.
7979

8080
**Bad:**
8181
```javascript
82-
// What the heck is 525600 for?
83-
for (let i = 0; i < 525600; i++) {
84-
runCronJob();
85-
}
82+
// What the heck is 86400 for?
83+
setTimeout(() => {
84+
this.blastOff()
85+
}, 86400);
86+
8687
```
8788

8889
**Good**:
8990
```javascript
9091
// Declare them as capitalized `const` globals.
91-
const MINUTES_IN_A_YEAR = 525600;
92-
for (let i = 0; i < MINUTES_IN_A_YEAR; i++) {
93-
runCronJob();
94-
}
92+
const SECONDS_IN_A_DAY = 86400;
93+
94+
setTimeout(() => {
95+
this.blastOff()
96+
}, SECONDS_IN_A_DAY);
97+
9598
```
9699
**[⬆ back to top](#table-of-contents)**
97100

98101
### Use explanatory variables
99102
**Bad:**
100103
```javascript
101-
const cityStateRegex = /^(.+)[,\\s]+(.+?)\s*(\d{5})?$/;
102-
saveCityState(cityStateRegex.match(cityStateRegex)[1], cityStateRegex.match(cityStateRegex)[2]);
104+
const address = 'One Infinite Loop, Cupertino 95014';
105+
const cityStateRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
106+
saveCityState(address.match(cityStateRegex)[1], address.match(cityStateRegex)[2]);
103107
```
104108

105109
**Good**:
106110
```javascript
107-
const cityStateRegex = /^(.+)[,\\s]+(.+?)\s*(\d{5})?$/;
108-
const match = cityStateRegex.match(cityStateRegex)
109-
const city = match[1];
110-
const state = match[2];
111+
const address = 'One Infinite Loop, Cupertino 95014';
112+
const cityStateRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
113+
const [, city, state] = address.match(cityStateRegex);
111114
saveCityState(city, state);
112115
```
113116
**[⬆ back to top](#table-of-contents)**
@@ -174,25 +177,23 @@ function paintCar(car) {
174177
```
175178
**[⬆ back to top](#table-of-contents)**
176179

177-
### Short-circuiting is cleaner than conditionals
180+
### Use default arguments instead of short circuiting or conditionals
178181

179182
**Bad:**
180183
```javascript
181184
function createMicrobrewery(name) {
182-
let breweryName;
183-
if (name) {
184-
breweryName = name;
185-
} else {
186-
breweryName = 'Hipster Brew Co.';
187-
}
185+
const breweryName = name || 'Hipster Brew Co.';
186+
...
188187
}
188+
189189
```
190190

191191
**Good**:
192192
```javascript
193-
function createMicrobrewery(name) {
194-
const breweryName = name || 'Hipster Brew Co.'
193+
function createMicrobrewery(breweryName = 'Hipster Brew Co.') {
194+
...
195195
}
196+
196197
```
197198
**[⬆ back to top](#table-of-contents)**
198199

@@ -227,7 +228,7 @@ const menuConfig = {
227228
body: 'Bar',
228229
buttonText: 'Baz',
229230
cancellable: true
230-
}
231+
};
231232

232233
function createMenu(config) {
233234
// ...
@@ -292,7 +293,7 @@ function addMonthToDate(month, date) {
292293
}
293294

294295
const date = new Date();
295-
addMonthToDate(date, 1);
296+
addMonthToDate(1, date);
296297
```
297298
**[⬆ back to top](#table-of-contents)**
298299

@@ -313,7 +314,7 @@ function parseBetterJSAlternative(code) {
313314
REGEXES.forEach((REGEX) => {
314315
statements.forEach((statement) => {
315316
// ...
316-
})
317+
});
317318
});
318319

319320
const ast = [];
@@ -323,7 +324,7 @@ function parseBetterJSAlternative(code) {
323324

324325
ast.forEach((node) => {
325326
// parse...
326-
})
327+
});
327328
}
328329
```
329330

@@ -339,7 +340,7 @@ function tokenize(code) {
339340
REGEXES.forEach((REGEX) => {
340341
statements.forEach((statement) => {
341342
tokens.push( /* ... */ );
342-
})
343+
});
343344
});
344345

345346
return tokens;
@@ -359,7 +360,7 @@ function parseBetterJSAlternative(code) {
359360
const ast = lexer(tokens);
360361
ast.forEach((node) => {
361362
// parse...
362-
})
363+
});
363364
}
364365
```
365366
**[⬆ back to top](#table-of-contents)**
@@ -431,25 +432,6 @@ function showList(employees) {
431432
```
432433
**[⬆ back to top](#table-of-contents)**
433434

434-
### Use default arguments instead of short circuiting
435-
**Bad:**
436-
```javascript
437-
function writeForumComment(subject, body) {
438-
subject = subject || 'No Subject';
439-
body = body || 'No text';
440-
}
441-
442-
```
443-
444-
**Good**:
445-
```javascript
446-
function writeForumComment(subject = 'No subject', body = 'No text') {
447-
// ...
448-
}
449-
450-
```
451-
**[⬆ back to top](#table-of-contents)**
452-
453435
### Set default objects with Object.assign
454436

455437
**Bad:**
@@ -459,12 +441,12 @@ const menuConfig = {
459441
body: 'Bar',
460442
buttonText: null,
461443
cancellable: true
462-
}
444+
};
463445

464446
function createMenu(config) {
465-
config.title = config.title || 'Foo'
466-
config.body = config.body || 'Bar'
467-
config.buttonText = config.buttonText || 'Baz'
447+
config.title = config.title || 'Foo';
448+
config.body = config.body || 'Bar';
449+
config.buttonText = config.buttonText || 'Baz';
468450
config.cancellable = config.cancellable === undefined ? config.cancellable : true;
469451

470452
}
@@ -479,7 +461,7 @@ const menuConfig = {
479461
// User did not include 'body' key
480462
buttonText: 'Send',
481463
cancellable: true
482-
}
464+
};
483465

484466
function createMenu(config) {
485467
config = Object.assign({
@@ -561,7 +543,7 @@ function splitIntoFirstAndLastName(name) {
561543
return name.split(' ');
562544
}
563545

564-
const name = 'Ryan McDermott'
546+
const name = 'Ryan McDermott';
565547
const newName = splitIntoFirstAndLastName(name);
566548

567549
console.log(name); // 'Ryan McDermott';
@@ -597,7 +579,7 @@ Array.prototype.diff = function diff(comparisonArray) {
597579
}
598580

599581
return values;
600-
}
582+
};
601583
```
602584

603585
**Good:**
@@ -903,13 +885,13 @@ using getters and setters to access data on objects is far better than simply
903885
looking for a property on an object. "Why?" you might ask. Well, here's an
904886
unorganized list of reasons why:
905887

906-
1. When you want to do more beyond getting an object property, you don't have
888+
* When you want to do more beyond getting an object property, you don't have
907889
to look up and change every accessor in your codebase.
908-
2. Makes adding validation simple when doing a `set`.
909-
3. Encapsulates the internal representation.
910-
4. Easy to add logging and error handling when getting and setting.
911-
5. Inheriting this class, you can override default functionality.
912-
6. You can lazy load your object's properties, let's say getting it from a
890+
* Makes adding validation simple when doing a `set`.
891+
* Encapsulates the internal representation.
892+
* Easy to add logging and error handling when getting and setting.
893+
* Inheriting this class, you can override default functionality.
894+
* You can lazy load your object's properties, let's say getting it from a
913895
server.
914896

915897

@@ -940,11 +922,11 @@ class BankAccount {
940922
this._balance = amount;
941923
}
942924
}
943-
925+
944926
get balance() {
945927
return this._balance;
946928
}
947-
929+
948930
verifyIfAmountCanBeSetted(val) {
949931
// ...
950932
}
@@ -970,11 +952,11 @@ This can be accomplished through closures (for ES5 and below).
970952

971953
const Employee = function(name) {
972954
this.name = name;
973-
}
955+
};
974956

975957
Employee.prototype.getName = function getName() {
976958
return this.name;
977-
}
959+
};
978960

979961
const employee = new Employee('John Doe');
980962
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
@@ -1044,7 +1026,7 @@ class UserAuth {
10441026
class UserSettings {
10451027
constructor(user) {
10461028
this.user = user;
1047-
this.auth = new UserAuth(user)
1029+
this.auth = new UserAuth(user);
10481030
}
10491031

10501032
changeSettings(settings) {
@@ -1163,7 +1145,7 @@ function renderLargeRectangles(rectangles) {
11631145
rectangle.setHeight(5);
11641146
const area = rectangle.getArea(); // BAD: Will return 25 for Square. Should be 20.
11651147
rectangle.render(area);
1166-
})
1148+
});
11671149
}
11681150

11691151
const rectangles = [new Rectangle(), new Rectangle(), new Square()];
@@ -1232,7 +1214,7 @@ function renderLargeShapes(shapes) {
12321214

12331215
const area = shape.getArea();
12341216
shape.render(area);
1235-
})
1217+
});
12361218
}
12371219

12381220
const shapes = [new Rectangle(), new Rectangle(), new Square()];
@@ -1523,7 +1505,7 @@ class Car {
15231505
const car = new Car();
15241506
car.setColor('pink');
15251507
car.setMake('Ford');
1526-
car.setModel('F-150')
1508+
car.setModel('F-150');
15271509
car.save();
15281510
```
15291511

@@ -1721,9 +1703,9 @@ require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (req
17211703
} else {
17221704
console.log('File written');
17231705
}
1724-
})
1706+
});
17251707
}
1726-
})
1708+
});
17271709

17281710
```
17291711

@@ -1738,7 +1720,7 @@ require('request-promise').get('https://en.wikipedia.org/wiki/Robert_Cecil_Marti
17381720
})
17391721
.catch((err) => {
17401722
console.error(err);
1741-
})
1723+
});
17421724

17431725
```
17441726
**[⬆ back to top](#table-of-contents)**
@@ -1761,15 +1743,15 @@ require('request-promise').get('https://en.wikipedia.org/wiki/Robert_Cecil_Marti
17611743
})
17621744
.catch((err) => {
17631745
console.error(err);
1764-
})
1746+
});
17651747

17661748
```
17671749

17681750
**Good**:
17691751
```javascript
17701752
async function getCleanCodeArticle() {
17711753
try {
1772-
const request = await require('request-promise')
1754+
const request = await require('request-promise');
17731755
const response = await request.get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin');
17741756
const fileHandle = await require('fs-promise');
17751757

@@ -2093,7 +2075,7 @@ $scope.model = {
20932075
////////////////////////////////////////////////////////////////////////////////
20942076
const actions = function() {
20952077
// ...
2096-
}
2078+
};
20972079
```
20982080

20992081
**Good**:
@@ -2105,6 +2087,6 @@ $scope.model = {
21052087

21062088
const actions = function() {
21072089
// ...
2108-
}
2090+
};
21092091
```
21102092
**[⬆ back to top](#table-of-contents)**

0 commit comments

Comments
 (0)