@@ -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**:
|
@@ -1002,12 +1005,12 @@ class UserSettings {
|
1002 | 1005 | }
|
1003 | 1006 |
|
1004 | 1007 | changeSettings(settings) {
|
1005 |
| -if (this.verifyCredentials(user)) { |
| 1008 | +if (this.verifyCredentials()) { |
1006 | 1009 | // ...
|
1007 | 1010 | }
|
1008 | 1011 | }
|
1009 | 1012 |
|
1010 |
| -verifyCredentials(user) { |
| 1013 | +verifyCredentials() { |
1011 | 1014 | // ...
|
1012 | 1015 | }
|
1013 | 1016 | }
|
@@ -1209,6 +1212,7 @@ function renderLargeShapes(shapes) {
|
1209 | 1212 | switch (shape.constructor.name) {
|
1210 | 1213 | case 'Square':
|
1211 | 1214 | shape.setLength(5);
|
| 1215 | +break; |
1212 | 1216 | case 'Rectangle':
|
1213 | 1217 | shape.setWidth(4);
|
1214 | 1218 | shape.setHeight(5);
|
@@ -1321,6 +1325,16 @@ example below, the implicit contract is that any Request module for an
|
1321 | 1325 |
|
1322 | 1326 | **Bad:**
|
1323 | 1327 | ```javascript
|
| 1328 | +class InventoryRequester { |
| 1329 | +constructor() { |
| 1330 | +this.REQ_METHODS = ['HTTP']; |
| 1331 | +} |
| 1332 | + |
| 1333 | +requestItem(item) { |
| 1334 | +// ... |
| 1335 | +} |
| 1336 | +} |
| 1337 | + |
1324 | 1338 | class InventoryTracker {
|
1325 | 1339 | constructor(items) {
|
1326 | 1340 | this.items = items;
|
@@ -1337,16 +1351,6 @@ class InventoryTracker {
|
1337 | 1351 | }
|
1338 | 1352 | }
|
1339 | 1353 |
|
1340 |
| -class InventoryRequester { |
1341 |
| -constructor() { |
1342 |
| -this.REQ_METHODS = ['HTTP']; |
1343 |
| -} |
1344 |
| - |
1345 |
| -requestItem(item) { |
1346 |
| -// ... |
1347 |
| -} |
1348 |
| -} |
1349 |
| - |
1350 | 1354 | const inventoryTracker = new InventoryTracker(['apples', 'bananas']);
|
1351 | 1355 | inventoryTracker.requestItems();
|
1352 | 1356 | ```
|
@@ -1402,35 +1406,35 @@ classes until you find yourself needing larger and more complex objects.
|
1402 | 1406 | **Bad:**
|
1403 | 1407 | ```javascript
|
1404 | 1408 | const Animal = function(age) {
|
1405 |
| -if (!(this instanceof Animal)) { |
1406 |
| -throw new Error("Instantiate Animal with `new`"); |
1407 |
| -} |
| 1409 | +if (!(this instanceof Animal)) { |
| 1410 | +throw new Error("Instantiate Animal with `new`"); |
| 1411 | +} |
1408 | 1412 |
|
1409 |
| -this.age = age; |
| 1413 | +this.age = age; |
1410 | 1414 | };
|
1411 | 1415 |
|
1412 | 1416 | Animal..move = function move() {};
|
1413 | 1417 |
|
1414 | 1418 | const Mammal = function(age, furColor) {
|
1415 |
| -if (!(this instanceof Mammal)) { |
1416 |
| -throw new Error("Instantiate Mammal with `new`"); |
1417 |
| -} |
| 1419 | +if (!(this instanceof Mammal)) { |
| 1420 | +throw new Error("Instantiate Mammal with `new`"); |
| 1421 | +} |
1418 | 1422 |
|
1419 |
| -Animal.call(this, age); |
1420 |
| -this.furColor = furColor; |
| 1423 | +Animal.call(this, age); |
| 1424 | +this.furColor = furColor; |
1421 | 1425 | };
|
1422 | 1426 |
|
1423 | 1427 | Mammal. = Object.create(Animal.);
|
1424 | 1428 | Mammal..constructor = Mammal;
|
1425 | 1429 | Mammal..liveBirth = function liveBirth() {};
|
1426 | 1430 |
|
1427 | 1431 | const Human = function(age, furColor, languageSpoken) {
|
1428 |
| -if (!(this instanceof Human)) { |
1429 |
| -throw new Error("Instantiate Human with `new`"); |
1430 |
| -} |
| 1432 | +if (!(this instanceof Human)) { |
| 1433 | +throw new Error("Instantiate Human with `new`"); |
| 1434 | +} |
1431 | 1435 |
|
1432 |
| -Mammal.call(this, age, furColor); |
1433 |
| -this.languageSpoken = languageSpoken; |
| 1436 | +Mammal.call(this, age, furColor); |
| 1437 | +this.languageSpoken = languageSpoken; |
1434 | 1438 | };
|
1435 | 1439 |
|
1436 | 1440 | Human. = Object.create(Mammal.);
|
@@ -1441,29 +1445,29 @@ Human..speak = function speak() {};
|
1441 | 1445 | **Good:**
|
1442 | 1446 | ```javascript
|
1443 | 1447 | class Animal {
|
1444 |
| -constructor(age) { |
1445 |
| -this.age = age; |
1446 |
| -} |
| 1448 | +constructor(age) { |
| 1449 | +this.age = age; |
| 1450 | +} |
1447 | 1451 |
|
1448 |
| -move() {} |
| 1452 | +move() { /* ... */ } |
1449 | 1453 | }
|
1450 | 1454 |
|
1451 | 1455 | class Mammal extends Animal {
|
1452 |
| -constructor(age, furColor) { |
1453 |
| -super(age); |
1454 |
| -this.furColor = furColor; |
1455 |
| -} |
| 1456 | +constructor(age, furColor) { |
| 1457 | +super(age); |
| 1458 | +this.furColor = furColor; |
| 1459 | +} |
1456 | 1460 |
|
1457 |
| -liveBirth() {} |
| 1461 | +liveBirth() { /* ... */ } |
1458 | 1462 | }
|
1459 | 1463 |
|
1460 | 1464 | class Human extends Mammal {
|
1461 |
| -constructor(age, furColor, languageSpoken) { |
1462 |
| -super(age, furColor); |
1463 |
| -this.languageSpoken = languageSpoken; |
1464 |
| -} |
| 1465 | +constructor(age, furColor, languageSpoken) { |
| 1466 | +super(age, furColor); |
| 1467 | +this.languageSpoken = languageSpoken; |
| 1468 | +} |
1465 | 1469 |
|
1466 |
| -speak() {} |
| 1470 | +speak() { /* ... */ } |
1467 | 1471 | }
|
1468 | 1472 | ```
|
1469 | 1473 | **[⬆ back to top](#table-of-contents)**
|
@@ -1596,6 +1600,15 @@ class EmployeeTaxData extends Employee {
|
1596 | 1600 |
|
1597 | 1601 | **Good**:
|
1598 | 1602 | ```javascript
|
| 1603 | +class EmployeeTaxData { |
| 1604 | +constructor(ssn, salary) { |
| 1605 | +this.ssn = ssn; |
| 1606 | +this.salary = salary; |
| 1607 | +} |
| 1608 | + |
| 1609 | +// ... |
| 1610 | +} |
| 1611 | + |
1599 | 1612 | class Employee {
|
1600 | 1613 | constructor(name, email) {
|
1601 | 1614 | this.name = name;
|
@@ -1608,15 +1621,6 @@ class Employee {
|
1608 | 1621 | }
|
1609 | 1622 | // ...
|
1610 | 1623 | }
|
1611 |
| - |
1612 |
| -class EmployeeTaxData { |
1613 |
| -constructor(ssn, salary) { |
1614 |
| -this.ssn = ssn; |
1615 |
| -this.salary = salary; |
1616 |
| -} |
1617 |
| - |
1618 |
| -// ... |
1619 |
| -} |
1620 | 1624 | ```
|
1621 | 1625 | **[⬆ back to top](#table-of-contents)**
|
1622 | 1626 |
|
@@ -1695,14 +1699,14 @@ Promises are a built-in global type. Use them!
|
1695 | 1699 |
|
1696 | 1700 | **Bad:**
|
1697 | 1701 | ```javascript
|
1698 |
| -require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (err, response) => { |
1699 |
| -if (err) { |
1700 |
| -console.error(err); |
| 1702 | +require('request').get('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', (requestErr, response) => { |
| 1703 | +if (requestErr) { |
| 1704 | +console.error(requestErr); |
1701 | 1705 | }
|
1702 | 1706 | else {
|
1703 |
| -require('fs').writeFile('article.html', response.body, (err) => { |
1704 |
| -if (err) { |
1705 |
| -console.error(err); |
| 1707 | +require('fs').writeFile('article.html', response.body, (writeErr) => { |
| 1708 | +if (writeErr) { |
| 1709 | +console.error(writeErr); |
1706 | 1710 | } else {
|
1707 | 1711 | console.log('File written');
|
1708 | 1712 | }
|
@@ -1993,7 +1997,7 @@ function hashIt(data) {
|
1993 | 1997 | // Make the hash
|
1994 | 1998 | hash = ((hash << 5) - hash) + char;
|
1995 | 1999 | // Convert to 32-bit integer
|
1996 |
| -hash = hash & hash; |
| 2000 | +hash &= hash; |
1997 | 2001 | }
|
1998 | 2002 | }
|
1999 | 2003 | ```
|
@@ -2010,7 +2014,7 @@ function hashIt(data) {
|
2010 | 2014 | hash = ((hash << 5) - hash) + char;
|
2011 | 2015 |
|
2012 | 2016 | // Convert to 32-bit integer
|
2013 |
| -hash = hash & hash; |
| 2017 | +hash &= hash; |
2014 | 2018 | }
|
2015 | 2019 | }
|
2016 | 2020 |
|
|
0 commit comments