@@ -72,7 +72,10 @@ getUser();
|
72 | 72 | We will read more code than we will ever write. It's important that the code we
|
73 | 73 | do write is readable and searchable. By *not* naming variables that end up
|
74 | 74 | being meaningful for understanding our program, we hurt our readers.
|
75 |
| -Make your names searchable. |
| 75 | +Make your names searchable. Tools like |
| 76 | +[buddy.js](https://.com/danielstjules/buddy.js) and |
| 77 | +[ESLint](https://.com/eslint/eslint/blob/660e0918933e6e7fede26bc675a0763a6b357c94/docs/rules/no-magic-numbers.md) |
| 78 | +can help identify unnamed constants. |
76 | 79 |
|
77 | 80 | **Bad:**
|
78 | 81 | ```javascript
|
@@ -226,7 +229,7 @@ const menuConfig = {
|
226 | 229 | cancellable: true
|
227 | 230 | }
|
228 | 231 |
|
229 |
| -function createMenu(menuConfig) { |
| 232 | +function createMenu(config) { |
230 | 233 | // ...
|
231 | 234 | }
|
232 | 235 |
|
@@ -367,7 +370,7 @@ for it and it's quite possibly the worst sin you can commit as a professional
|
367 | 370 | developer. Duplicate code means there's more than one place to alter something
|
368 | 371 | if you need to change some logic. JavaScript is untyped, so it makes having
|
369 | 372 | generic functions quite easy. Take advantage of that! Tools like
|
370 |
| -[jsinpect](https://.com/danielstjules/jsinspect) can help you find duplicate |
| 373 | +[jsinspect](https://.com/danielstjules/jsinspect) can help you find duplicate |
371 | 374 | code eligible for refactoring.
|
372 | 375 |
|
373 | 376 | **Bad:**
|
@@ -741,11 +744,11 @@ class Airplane {
|
741 | 744 | getCruisingAltitude() {
|
742 | 745 | switch (this.type) {
|
743 | 746 | case '777':
|
744 |
| -return getMaxAltitude() - getPassengerCount(); |
| 747 | +return this.getMaxAltitude() - this.getPassengerCount(); |
745 | 748 | case 'Air Force One':
|
746 |
| -return getMaxAltitude(); |
| 749 | +return this.getMaxAltitude(); |
747 | 750 | case 'Cessna':
|
748 |
| -return getMaxAltitude() - getFuelExpenditure(); |
| 751 | +return this.getMaxAltitude() - this.getFuelExpenditure(); |
749 | 752 | }
|
750 | 753 | }
|
751 | 754 | }
|
@@ -760,21 +763,21 @@ class Airplane {
|
760 | 763 | class Boeing777 extends Airplane {
|
761 | 764 | // ...
|
762 | 765 | getCruisingAltitude() {
|
763 |
| -return getMaxAltitude() - getPassengerCount(); |
| 766 | +return this.getMaxAltitude() - this.getPassengerCount(); |
764 | 767 | }
|
765 | 768 | }
|
766 | 769 |
|
767 | 770 | class AirForceOne extends Airplane {
|
768 | 771 | // ...
|
769 | 772 | getCruisingAltitude() {
|
770 |
| -return getMaxAltitude(); |
| 773 | +return this.getMaxAltitude(); |
771 | 774 | }
|
772 | 775 | }
|
773 | 776 |
|
774 | 777 | class Cessna extends Airplane {
|
775 | 778 | // ...
|
776 | 779 | getCruisingAltitude() {
|
777 |
| -return getMaxAltitude() - getFuelExpenditure(); |
| 780 | +return this.getMaxAltitude() - this.getFuelExpenditure(); |
778 | 781 | }
|
779 | 782 | }
|
780 | 783 | ```
|
@@ -819,12 +822,12 @@ TypeScript (which, like I said, is a great alternative!).
|
819 | 822 | **Bad:**
|
820 | 823 | ```javascript
|
821 | 824 | function combine(val1, val2) {
|
822 |
| -if (typeof val1 == "number" && typeof val2 == "number" || |
823 |
| -typeof val1 == "string" && typeof val2 == "string") { |
| 825 | +if (typeof val1 === 'number' && typeof val2 === 'number' || |
| 826 | +typeof val1 === 'string' && typeof val2 === 'string') { |
824 | 827 | return val1 + val2;
|
825 |
| -} else { |
826 |
| -throw new Error('Must be of type String or Number'); |
827 | 828 | }
|
| 829 | + |
| 830 | +throw new Error('Must be of type String or Number'); |
828 | 831 | }
|
829 | 832 | ```
|
830 | 833 |
|
@@ -921,7 +924,7 @@ class BankAccount {
|
921 | 924 | const bankAccount = new BankAccount();
|
922 | 925 |
|
923 | 926 | // Buy shoes...
|
924 |
| -bankAccount.balance = bankAccount.balance - 100; |
| 927 | +bankAccount.balance -= 100; |
925 | 928 | ```
|
926 | 929 |
|
927 | 930 | **Good**:
|
@@ -1006,12 +1009,12 @@ class UserSettings {
|
1006 | 1009 | }
|
1007 | 1010 |
|
1008 | 1011 | changeSettings(settings) {
|
1009 |
| -if (this.verifyCredentials(user)) { |
| 1012 | +if (this.verifyCredentials()) { |
1010 | 1013 | // ...
|
1011 | 1014 | }
|
1012 | 1015 | }
|
1013 | 1016 |
|
1014 |
| -verifyCredentials(user) { |
| 1017 | +verifyCredentials() { |
1015 | 1018 | // ...
|
1016 | 1019 | }
|
1017 | 1020 | }
|
@@ -1213,6 +1216,7 @@ function renderLargeShapes(shapes) {
|
1213 | 1216 | switch (shape.constructor.name) {
|
1214 | 1217 | case 'Square':
|
1215 | 1218 | shape.setLength(5);
|
| 1219 | +break; |
1216 | 1220 | case 'Rectangle':
|
1217 | 1221 | shape.setWidth(4);
|
1218 | 1222 | shape.setHeight(5);
|
@@ -1325,6 +1329,16 @@ example below, the implicit contract is that any Request module for an
|
1325 | 1329 |
|
1326 | 1330 | **Bad:**
|
1327 | 1331 | ```javascript
|
| 1332 | +class InventoryRequester { |
| 1333 | +constructor() { |
| 1334 | +this.REQ_METHODS = ['HTTP']; |
| 1335 | +} |
| 1336 | + |
| 1337 | +requestItem(item) { |
| 1338 | +// ... |
| 1339 | +} |
| 1340 | +} |
| 1341 | + |
1328 | 1342 | class InventoryTracker {
|
1329 | 1343 | constructor(items) {
|
1330 | 1344 | this.items = items;
|
@@ -1341,16 +1355,6 @@ class InventoryTracker {
|
1341 | 1355 | }
|
1342 | 1356 | }
|
1343 | 1357 |
|
1344 |
| -class InventoryRequester { |
1345 |
| -constructor() { |
1346 |
| -this.REQ_METHODS = ['HTTP']; |
1347 |
| -} |
1348 |
| - |
1349 |
| -requestItem(item) { |
1350 |
| -// ... |
1351 |
| -} |
1352 |
| -} |
1353 |
| - |
1354 | 1358 | const inventoryTracker = new InventoryTracker(['apples', 'bananas']);
|
1355 | 1359 | inventoryTracker.requestItems();
|
1356 | 1360 | ```
|
@@ -1406,35 +1410,35 @@ classes until you find yourself needing larger and more complex objects.
|
1406 | 1410 | **Bad:**
|
1407 | 1411 | ```javascript
|
1408 | 1412 | const Animal = function(age) {
|
1409 |
| -if (!(this instanceof Animal)) { |
1410 |
| -throw new Error("Instantiate Animal with `new`"); |
1411 |
| -} |
| 1413 | +if (!(this instanceof Animal)) { |
| 1414 | +throw new Error("Instantiate Animal with `new`"); |
| 1415 | +} |
1412 | 1416 |
|
1413 |
| -this.age = age; |
| 1417 | +this.age = age; |
1414 | 1418 | };
|
1415 | 1419 |
|
1416 | 1420 | Animal..move = function() {};
|
1417 | 1421 |
|
1418 | 1422 | const Mammal = function(age, furColor) {
|
1419 |
| -if (!(this instanceof Mammal)) { |
1420 |
| -throw new Error("Instantiate Mammal with `new`"); |
1421 |
| -} |
| 1423 | +if (!(this instanceof Mammal)) { |
| 1424 | +throw new Error("Instantiate Mammal with `new`"); |
| 1425 | +} |
1422 | 1426 |
|
1423 |
| -Animal.call(this, age); |
1424 |
| -this.furColor = furColor; |
| 1427 | +Animal.call(this, age); |
| 1428 | +this.furColor = furColor; |
1425 | 1429 | };
|
1426 | 1430 |
|
1427 | 1431 | Mammal. = Object.create(Animal.);
|
1428 | 1432 | Mammal..constructor = Mammal;
|
1429 | 1433 | Mammal..liveBirth = function() {};
|
1430 | 1434 |
|
1431 | 1435 | const Human = function(age, furColor, languageSpoken) {
|
1432 |
| -if (!(this instanceof Human)) { |
1433 |
| -throw new Error("Instantiate Human with `new`"); |
1434 |
| -} |
| 1436 | +if (!(this instanceof Human)) { |
| 1437 | +throw new Error("Instantiate Human with `new`"); |
| 1438 | +} |
1435 | 1439 |
|
1436 |
| -Mammal.call(this, age, furColor); |
1437 |
| -this.languageSpoken = languageSpoken; |
| 1440 | +Mammal.call(this, age, furColor); |
| 1441 | +this.languageSpoken = languageSpoken; |
1438 | 1442 | };
|
1439 | 1443 |
|
1440 | 1444 | Human. = Object.create(Mammal.);
|
@@ -1445,29 +1449,29 @@ Human..speak = function() {};
|
1445 | 1449 | **Good:**
|
1446 | 1450 | ```javascript
|
1447 | 1451 | class Animal {
|
1448 |
| -constructor(age) { |
1449 |
| -this.age = age; |
1450 |
| -} |
| 1452 | +constructor(age) { |
| 1453 | +this.age = age; |
| 1454 | +} |
1451 | 1455 |
|
1452 |
| -move() {} |
| 1456 | +move() { /* ... */ } |
1453 | 1457 | }
|
1454 | 1458 |
|
1455 | 1459 | class Mammal extends Animal {
|
1456 |
| -constructor(age, furColor) { |
1457 |
| -super(age); |
1458 |
| -this.furColor = furColor; |
1459 |
| -} |
| 1460 | +constructor(age, furColor) { |
| 1461 | +super(age); |
| 1462 | +this.furColor = furColor; |
| 1463 | +} |
1460 | 1464 |
|
1461 |
| -liveBirth() {} |
| 1465 | +liveBirth() { /* ... */ } |
1462 | 1466 | }
|
1463 | 1467 |
|
1464 | 1468 | class Human extends Mammal {
|
1465 |
| -constructor(age, furColor, languageSpoken) { |
1466 |
| -super(age, furColor); |
1467 |
| -this.languageSpoken = languageSpoken; |
1468 |
| -} |
| 1469 | +constructor(age, furColor, languageSpoken) { |
| 1470 | +super(age, furColor); |
| 1471 | +this.languageSpoken = languageSpoken; |
| 1472 | +} |
1469 | 1473 |
|
1470 |
| -speak() {} |
| 1474 | +speak() { /* ... */ } |
1471 | 1475 | }
|
1472 | 1476 | ```
|
1473 | 1477 | **[⬆ back to top](#table-of-contents)**
|
@@ -1600,6 +1604,15 @@ class EmployeeTaxData extends Employee {
|
1600 | 1604 |
|
1601 | 1605 | **Good**:
|
1602 | 1606 | ```javascript
|
| 1607 | +class EmployeeTaxData { |
| 1608 | +constructor(ssn, salary) { |
| 1609 | +this.ssn = ssn; |
| 1610 | +this.salary = salary; |
| 1611 | +} |
| 1612 | + |
| 1613 | +// ... |
| 1614 | +} |
| 1615 | + |
1603 | 1616 | class Employee {
|
1604 | 1617 | constructor(name, email) {
|
1605 | 1618 | this.name = name;
|
@@ -1612,15 +1625,6 @@ class Employee {
|
1612 | 1625 | }
|
1613 | 1626 | // ...
|
1614 | 1627 | }
|
1615 |
| - |
1616 |
| -class EmployeeTaxData { |
1617 |
| -constructor(ssn, salary) { |
1618 |
| -this.ssn = ssn; |
1619 |
| -this.salary = salary; |
1620 |
| -} |
1621 |
| - |
1622 |
| -// ... |
1623 |
| -} |
1624 | 1628 | ```
|
1625 | 1629 | **[⬆ back to top](#table-of-contents)**
|
1626 | 1630 |
|
@@ -1699,13 +1703,13 @@ Promises are a built-in global type. Use them!
|
1699 | 1703 |
|
1700 | 1704 | **Bad:**
|
1701 | 1705 | ```javascript
|
1702 |
| -require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', function(err, response) { |
1703 |
| -if (err) { |
1704 |
| -console.error(err); |
| 1706 | +require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (requestErr, response) => { |
| 1707 | +if (requestErr) { |
| 1708 | +console.error(requestErr); |
1705 | 1709 | } else {
|
1706 |
| -require('fs').writeFile('article.html', response.body, function(err) { |
1707 |
| -if (err) { |
1708 |
| -console.error(err); |
| 1710 | +require('fs').writeFile('article.html', response.body, (writeErr) => { |
| 1711 | +if (writeErr) { |
| 1712 | +console.error(writeErr); |
1709 | 1713 | } else {
|
1710 | 1714 | console.log('File written');
|
1711 | 1715 | }
|
@@ -1996,7 +2000,7 @@ function hashIt(data) {
|
1996 | 2000 | // Make the hash
|
1997 | 2001 | hash = ((hash << 5) - hash) + char;
|
1998 | 2002 | // Convert to 32-bit integer
|
1999 |
| -hash = hash & hash; |
| 2003 | +hash &= hash; |
2000 | 2004 | }
|
2001 | 2005 | }
|
2002 | 2006 | ```
|
@@ -2013,7 +2017,7 @@ function hashIt(data) {
|
2013 | 2017 | hash = ((hash << 5) - hash) + char;
|
2014 | 2018 |
|
2015 | 2019 | // Convert to 32-bit integer
|
2016 |
| -hash = hash & hash; |
| 2020 | +hash &= hash; |
2017 | 2021 | }
|
2018 | 2022 | }
|
2019 | 2023 |
|
|
0 commit comments