From a9894e9c07b54ac9e5653187d43d83bcee25f7da Mon Sep 17 00:00:00 2001 From: David Vujic Date: Wed, 11 Jan 2017 16:22:30 +0100 Subject: [PATCH 01/52] Functions are often cleaner than subclasses --- README.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/README.md b/README.md index 4f8ca461..01426880 100644 --- a/README.md +++ b/README.md @@ -609,6 +609,35 @@ class SuperArray extends Array { ``` **[⬆ back to top](#table-of-contents)** +### Functions are often cleaner than subclasses ### +Classes are good when you need state. Consider writing simple functions if state is not an issue. A drawback with a class extending the Array global, is that you are required to instantiate it using the 'new' keyword. Normally an array is created with: + +```javascript +const arr = []; +``` + +A class will limit the usage of the diff feature to the subclass only. A better option is to write a stateless function, that accept two arrays and calculate the difference between them. + +```javascript +function diff(first, second) { + const values = []; + const hash = {}; + + for (const i of second) { + hash[i] = true; + } + + for (const i of first) { + if (!hash[i]) { + values.push(i); + } + } + + return values; +} +``` +**[⬆ back to top](#table-of-contents)** + ### Favor functional programming over imperative programming JavaScript isn't a functional language in the way that Haskell is, but it has a functional flavor to it. Functional languages are cleaner and easier to test. From 7ea0e67cc71ef31ae3539a6044bdc494d6266af3 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 12 Jan 2017 09:20:00 +0100 Subject: [PATCH 02/52] diff code same as updated previous example --- README.md | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 4f801343..677907de 100644 --- a/README.md +++ b/README.md @@ -591,31 +591,18 @@ class SuperArray extends Array { ``` **[⬆ back to top](#table-of-contents)** -### Functions are often cleaner than subclasses ### +### Functions are cleaner than subclasses ### Classes are good when you need state. Consider writing simple functions if state is not an issue. A drawback with a class extending the Array global, is that you are required to instantiate it using the 'new' keyword. Normally an array is created with: ```javascript -const arr = []; +const arr = []; ``` A class will limit the usage of the diff feature to the subclass only. A better option is to write a stateless function, that accept two arrays and calculate the difference between them. ```javascript function diff(first, second) { - const values = []; - const hash = {}; - - for (const i of second) { - hash[i] = true; - } - - for (const i of first) { - if (!hash[i]) { - values.push(i); - } - } - - return values; + return first.filter(elem => !second.includes(elem)); } ``` **[⬆ back to top](#table-of-contents)** From c3fd7693472cc62c9a5272982fd1ba87a18e1561 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 12 Jan 2017 09:33:17 +0100 Subject: [PATCH 03/52] refactor: rewrite text, use Bad/Good format --- README.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 677907de..b64fd70a 100644 --- a/README.md +++ b/README.md @@ -592,18 +592,26 @@ class SuperArray extends Array { **[⬆ back to top](#table-of-contents)** ### Functions are cleaner than subclasses ### -Classes are good when you need state. Consider writing simple functions if state is not an issue. A drawback with a class extending the Array global, is that you are required to instantiate it using the 'new' keyword. Normally an array is created with: +Classes are good when you need state. Consider writing a simple function if state is not an issue. Also, when extending an object (like the global Array), you are required to instantiate it using the 'new' keyword. A class will limit the usage of a feature to the subclass only. A better option is to write a stateless function. +**Bad:** ```javascript -const arr = []; -``` +class SuperArray extends Array { + diff(comparisonArray) { + return this.filter(elem => !comparisonArray.includes(elem)); + } +} -A class will limit the usage of the diff feature to the subclass only. A better option is to write a stateless function, that accept two arrays and calculate the difference between them. +const arr = new SuperArray(1, 2, 3); +``` +**Good:** ```javascript function diff(first, second) { return first.filter(elem => !second.includes(elem)); } + +const arr = [1, 2, 3]; ``` **[⬆ back to top](#table-of-contents)** From 5d54b58adb25721dff36b4111be7690a084985f5 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 12 Jan 2017 09:34:38 +0100 Subject: [PATCH 04/52] refactor: rewrite text --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b64fd70a..5da1a310 100644 --- a/README.md +++ b/README.md @@ -592,7 +592,7 @@ class SuperArray extends Array { **[⬆ back to top](#table-of-contents)** ### Functions are cleaner than subclasses ### -Classes are good when you need state. Consider writing a simple function if state is not an issue. Also, when extending an object (like the global Array), you are required to instantiate it using the 'new' keyword. A class will limit the usage of a feature to the subclass only. A better option is to write a stateless function. +Classes are good when you need state. Consider writing a simple function if state is not an issue. Also, when extending an object (like the global Array), you are required to instantiate it using the 'new' keyword. A class will also limit the usage of a feature to the subclass only. A better option is to write a stateless function. **Bad:** ```javascript From 8e52681e3ef865097c7f861288249e7cbb362ad7 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 14 Jan 2017 12:09:11 +0100 Subject: [PATCH 05/52] updated code example after merge from master --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6b03f51f..cf8b8776 100644 --- a/README.md +++ b/README.md @@ -587,7 +587,8 @@ Classes are good when you need state. Consider writing a simple function if stat ```javascript class SuperArray extends Array { diff(comparisonArray) { - return this.filter(elem => !comparisonArray.includes(elem)); + const hash = new Set(comparisonArray); + return this.filter(elem => !hash.has(elem)); } } @@ -597,7 +598,8 @@ const arr = new SuperArray(1, 2, 3); **Good:** ```javascript function diff(first, second) { - return first.filter(elem => !second.includes(elem)); + const hash = new Set(second); + return first.filter(elem => !hash.has(elem)); } const arr = [1, 2, 3]; From 77acb99c72c6e1d771d2063ff8cdb9d6036b684c Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 17 Jan 2017 15:36:48 +0100 Subject: [PATCH 06/52] class extending a global object is bad --- README.md | 37 +++++++------------------------------ 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index cf8b8776..0b375520 100644 --- a/README.md +++ b/README.md @@ -558,8 +558,13 @@ extend JavaScript's native Array method to have a `diff` method that could show the difference between two arrays? You could write your new function to the `Array.prototype`, but it could clash with another library that tried to do the same thing. What if that other library was just using `diff` to find -the difference between the first and last elements of an array? This is why it -would be much better to just use ES2015/ES6 classes and simply extend the `Array` global. +the difference between the first and last elements of an array? + +One option is to use ES2015/ES6 classes. Classes are useful when you need state. +Consider writing a simple function if state is not an issue. Also, when extending an object, +features will be limited to instances of that subclass only. Another drawback is +(especially when extending built in globals like Array) that instances must be +created using the 'new' keyword. A better option is to write a stateless function. **Bad:** ```javascript @@ -569,40 +574,12 @@ Array.prototype.diff = function diff(comparisonArray) { }; ``` -**Good:** -```javascript -class SuperArray extends Array { - diff(comparisonArray) { - const hash = new Set(comparisonArray); - return this.filter(elem => !hash.has(elem)); - } -} -``` -**[⬆ back to top](#table-of-contents)** - -### Functions are cleaner than subclasses ### -Classes are good when you need state. Consider writing a simple function if state is not an issue. Also, when extending an object (like the global Array), you are required to instantiate it using the 'new' keyword. A class will also limit the usage of a feature to the subclass only. A better option is to write a stateless function. - -**Bad:** -```javascript -class SuperArray extends Array { - diff(comparisonArray) { - const hash = new Set(comparisonArray); - return this.filter(elem => !hash.has(elem)); - } -} - -const arr = new SuperArray(1, 2, 3); -``` - **Good:** ```javascript function diff(first, second) { const hash = new Set(second); return first.filter(elem => !hash.has(elem)); } - -const arr = [1, 2, 3]; ``` **[⬆ back to top](#table-of-contents)** From 11d2a8baa57cde8f104eecb0a1ab5111bb8f64a9 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 13:03:24 +0100 Subject: [PATCH 07/52] Module instead of Class --- README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d76d6439..04960f22 100644 --- a/README.md +++ b/README.md @@ -1013,13 +1013,17 @@ console.log(`Employee name: ${employee.getName()}`); // Employee name: undefined **Good:** ```javascript -const Employee = function (name) { - this.getName = function getName() { - return name; - }; -}; +function makeEmployee(name) { + function getName() { + return name; + } -const employee = new Employee('John Doe'); + return { + getName + }; +} + +const employee = makeEmployee('John Doe'); console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe delete employee.name; console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe From 9a2138e5a9a685e4884e90631dcbd2b28f3f1165 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 13:48:57 +0100 Subject: [PATCH 08/52] composed objects instead of classes --- README.md | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 056dbac6..a110b1d6 100644 --- a/README.md +++ b/README.md @@ -1676,27 +1676,31 @@ class EmployeeTaxData extends Employee { **Good:** ```javascript -class EmployeeTaxData { - constructor(ssn, salary) { - this.ssn = ssn; - this.salary = salary; - } - - // ... +function makeTaxData(ssn, salary) { + return { + ssn, + salary + }; } -class Employee { - constructor(name, email) { - this.name = name; - this.email = email; - } +function makeEmployee(name, email) { + let taxData = {}; - setTaxData(ssn, salary) { - this.taxData = new EmployeeTaxData(ssn, salary); - } - // ... -} -``` + function setTaxData(ssn, salary) { + taxData = makeTaxData(); + } + + function getTaxData() { + return taxData; + } + + return { + name, + email, + setTaxData, + getTaxData + } +}``` **[⬆ back to top](#table-of-contents)** ## **Testing** From 23629ec4970f6addac288c5daa0fa9c67b01844a Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 13:50:10 +0100 Subject: [PATCH 09/52] fix: typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a110b1d6..1a4b2668 100644 --- a/README.md +++ b/README.md @@ -1699,7 +1699,7 @@ function makeEmployee(name, email) { email, setTaxData, getTaxData - } + }; }``` **[⬆ back to top](#table-of-contents)** From 9f53ba04c2091a02b4b27be05518e0543521d646 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 14:11:49 +0100 Subject: [PATCH 10/52] refactor: fewer lines by appending an object --- README.md | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1a4b2668..4fb3b6c6 100644 --- a/README.md +++ b/README.md @@ -1684,22 +1684,16 @@ function makeTaxData(ssn, salary) { } function makeEmployee(name, email) { - let taxData = {}; - - function setTaxData(ssn, salary) { - taxData = makeTaxData(); - } + let obj = { + name, + email + }; - function getTaxData() { - return taxData; + obj.setTaxData = function (ssn, salary) { + obj.taxData = makeTaxData(ssn, salary); } - return { - name, - email, - setTaxData, - getTaxData - }; + return obj; }``` **[⬆ back to top](#table-of-contents)** From dd824ca0862fe66645a8419fec10ad82a48be104 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 14:41:32 +0100 Subject: [PATCH 11/52] fix: indendation, missing semicolon and line break --- README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 4fb3b6c6..d8057e99 100644 --- a/README.md +++ b/README.md @@ -1685,16 +1685,17 @@ function makeTaxData(ssn, salary) { function makeEmployee(name, email) { let obj = { - name, - email + name, + email }; obj.setTaxData = function (ssn, salary) { obj.taxData = makeTaxData(ssn, salary); - } + }; return obj; -}``` +} +``` **[⬆ back to top](#table-of-contents)** ## **Testing** From 7cf798dfc37c4cc764ba54c5d342707cb8c0569a Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 14:43:45 +0100 Subject: [PATCH 12/52] fix: const instead of let --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d8057e99..cfb6aab4 100644 --- a/README.md +++ b/README.md @@ -1684,7 +1684,7 @@ function makeTaxData(ssn, salary) { } function makeEmployee(name, email) { - let obj = { + const obj = { name, email }; From f7bdd8b0f7da646647192c9a808d1a15b76d2776 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 24 Jan 2017 15:06:59 +0100 Subject: [PATCH 13/52] fix: indentation --- README.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index cfb6aab4..f770b14d 100644 --- a/README.md +++ b/README.md @@ -1677,23 +1677,23 @@ class EmployeeTaxData extends Employee { **Good:** ```javascript function makeTaxData(ssn, salary) { - return { - ssn, - salary - }; + return { + ssn, + salary + }; } function makeEmployee(name, email) { - const obj = { - name, - email - }; + const obj = { + name, + email + }; - obj.setTaxData = function (ssn, salary) { - obj.taxData = makeTaxData(ssn, salary); - }; + obj.setTaxData = function (ssn, salary) { + obj.taxData = makeTaxData(ssn, salary); + }; - return obj; + return obj; } ``` **[⬆ back to top](#table-of-contents)** From 06c20790a6df5380010bd8370117a46373de5c02 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 15:48:42 +0100 Subject: [PATCH 14/52] Renamed Classes section to SOLID. Classes in a new sub section --- README.md | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index cebc95fe..86340340 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ 2. [Variables](#variables) 3. [Functions](#functions) 4. [Objects and Data Structures](#objects-and-data-structures) - 5. [Classes](#classes) + 5. [SOLID](#SOLID) 6. [Testing](#testing) 7. [Concurrency](#concurrency) 8. [Error Handling](#error-handling) @@ -395,7 +395,7 @@ duplicate code means creating an abstraction that can handle this set of different things with just one function/module/class. Getting the abstraction right is critical, that's why you should follow the -SOLID principles laid out in the *Classes* section. Bad abstractions can be +principles laid out in the *SOLID* section. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make a good abstraction, do it! Don't repeat yourself, otherwise you'll find yourself updating multiple places anytime you want to change one thing. @@ -1030,14 +1030,25 @@ console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe **[⬆ back to top](#table-of-contents)** -## **Classes** +### Classes +Classes are useful when you need state. Consider writing a function if state is not an issue. +Avoid creating potentially complex hierarchies by extending classes, +unless you are using a library like React and need a component using state (by extending React.Component). +Another thing to have in mind is, when extending classes, features will be limited to instances +of that subclass only. When extending built in globals like Array, instances must be created using +the 'new' keyword and cannot use shorthand syntax like `const arr = [];`. +A better option could be to write a stateless function. + +**[⬆ back to top](#table-of-contents)** + + +## **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 +It's tempting to jam-pack a class or a module 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, +that your class or module won't be conceptually cohesive and it will give it many reasons +to change. Minimizing the amount of times you need to change code is important. +It's important because if too much functionality is in one class or module and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase. From 9c1fd179ada205d20e6c7d6f94070995aa659218 Mon Sep 17 00:00:00 2001 From: David Vujic Date: Thu, 26 Jan 2017 15:56:22 +0100 Subject: [PATCH 15/52] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d86b47e2..80e85844 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ 2. [Variables](#variables) 3. [Functions](#functions) 4. [Objects and Data Structures](#objects-and-data-structures) - 5. [SOLID](#SOLID) + 5. [SOLID](#solid) 6. [Testing](#testing) 7. [Concurrency](#concurrency) 8. [Error Handling](#error-handling) From f589bfd07129f3189f53ca38292fde792072149a Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 15:57:42 +0100 Subject: [PATCH 16/52] fix: anchor to section --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 86340340..9e8cdbc6 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ 2. [Variables](#variables) 3. [Functions](#functions) 4. [Objects and Data Structures](#objects-and-data-structures) - 5. [SOLID](#SOLID) + 5. [SOLID](#solid) 6. [Testing](#testing) 7. [Concurrency](#concurrency) 8. [Error Handling](#error-handling) From ea52e230ccd863a41e5fc39ad1ca48d52e3d8dfc Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 16:08:30 +0100 Subject: [PATCH 17/52] refactor: from classes to stateless functions in modules --- README.md | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index cebc95fe..4fb23657 100644 --- a/README.md +++ b/README.md @@ -1062,29 +1062,24 @@ class UserSettings { **Good:** ```javascript -class UserAuth { - constructor(user) { - this.user = user; - } - - verifyCredentials() { +// this is the module userAuth.js +function verifyCredentials(user) { // ... - } } +export default verifyCredentials; -class UserSettings { - constructor(user) { - this.user = user; - this.auth = new UserAuth(user); - } - changeSettings(settings) { - if (this.auth.verifyCredentials()) { - // ... +// this is the module userSettings.js +import verifyCredentials from 'userAuth'; + +function changeSettings(user, settings) { + if (verifyCredentials(user)) { + // ... } - } } + +export default changeSettings; ``` **[⬆ back to top](#table-of-contents)** From 533918feb3e375ca04dab8fafc7250fbfec1cb28 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 16:09:11 +0100 Subject: [PATCH 18/52] refactor: from classes to stateless functions in modules --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4fb23657..88f212f2 100644 --- a/README.md +++ b/README.md @@ -1062,7 +1062,7 @@ class UserSettings { **Good:** ```javascript -// this is the module userAuth.js +// the module userAuth.js function verifyCredentials(user) { // ... } @@ -1070,8 +1070,8 @@ function verifyCredentials(user) { export default verifyCredentials; -// this is the module userSettings.js -import verifyCredentials from 'userAuth'; +// the module userSettings.js +import verifyCredentials from 'userAuth.js'; function changeSettings(user, settings) { if (verifyCredentials(user)) { From e81bbb08e0d688abdd0490d6d7d0cc56b3d703b4 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 17:36:34 +0100 Subject: [PATCH 19/52] Classes to separate section --- README.md | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 9e8cdbc6..3b33734a 100644 --- a/README.md +++ b/README.md @@ -7,12 +7,13 @@ 3. [Functions](#functions) 4. [Objects and Data Structures](#objects-and-data-structures) 5. [SOLID](#solid) - 6. [Testing](#testing) - 7. [Concurrency](#concurrency) - 8. [Error Handling](#error-handling) - 9. [Formatting](#formatting) - 10. [Comments](#comments) - 11. [Translation](#translation) + 6. [Classes](#classes) + 7. [Testing](#testing) + 8. [Concurrency](#concurrency) + 9. [Error Handling](#error-handling) + 10. [Formatting](#formatting) + 11. [Comments](#comments) + 12. [Translation](#translation) ## Introduction ![Humorous image of software quality estimation as a count of how many expletives @@ -1030,18 +1031,6 @@ console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe **[⬆ back to top](#table-of-contents)** -### Classes -Classes are useful when you need state. Consider writing a function if state is not an issue. -Avoid creating potentially complex hierarchies by extending classes, -unless you are using a library like React and need a component using state (by extending React.Component). -Another thing to have in mind is, when extending classes, features will be limited to instances -of that subclass only. When extending built in globals like Array, instances must be created using -the 'new' keyword and cannot use shorthand syntax like `const arr = [];`. -A better option could be to write a stateless function. - -**[⬆ back to top](#table-of-contents)** - - ## **SOLID** ### Single Responsibility Principle (SRP) It's tempting to jam-pack a class or a module with a lot of functionality, like @@ -1492,6 +1481,17 @@ inventoryTracker.requestItems(); ``` **[⬆ back to top](#table-of-contents)** + +## **Classes** +Classes are useful when you need state. Consider writing a function if state is not an issue. +Avoid creating potentially complex hierarchies by extending classes, +unless you are using a library like React and need a component using state (by extending React.Component). +Another thing to have in mind is, when extending classes, features will be limited to instances +of that subclass only. When extending built in globals like Array, instances must be created using +the 'new' keyword and cannot use shorthand syntax like `const arr = [];`. +A better option could be to write a stateless function. + + ### Prefer ES2015/ES6 classes over ES5 plain functions It's very difficult to get readable class inheritance, construction, and method definitions for classical ES5 classes. If you need inheritance (and be aware From 0e861a173b76dc470c2b2ff70ec176350804ad09 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 26 Jan 2017 19:06:04 +0100 Subject: [PATCH 20/52] rename credentials validator to a return-bool-like name --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 88f212f2..e0f79c64 100644 --- a/README.md +++ b/README.md @@ -1049,12 +1049,12 @@ class UserSettings { } changeSettings(settings) { - if (this.verifyCredentials()) { + if (this.hasValidCredentials()) { // ... } } - verifyCredentials() { + hasValidCredentials() { // ... } } @@ -1063,18 +1063,18 @@ class UserSettings { **Good:** ```javascript // the module userAuth.js -function verifyCredentials(user) { +function hasValidCredentials(user) { // ... } -export default verifyCredentials; +export default hasValidCredentials; // the module userSettings.js -import verifyCredentials from 'userAuth.js'; +import hasValidCredentials from 'userAuth.js'; function changeSettings(user, settings) { - if (verifyCredentials(user)) { + if (hasValidCredentials(user)) { // ... } } From f358218bb09998865bc94026f923cb6db085d53b Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 13:43:49 +0100 Subject: [PATCH 21/52] davids func remix --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a28d15aa..dc5bd565 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ -# clean-code-javascript +# clean-code-javascript (Davids func remix) ## Table of Contents 1. [Introduction](#introduction) From 5787001366709e368df829d11bd91027e46dbdc5 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 14:15:18 +0100 Subject: [PATCH 22/52] section Avoid conditionals as functions instead of classes --- README.md | 51 +++++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index dc5bd565..ba80e296 100644 --- a/README.md +++ b/README.md @@ -773,46 +773,53 @@ just do one thing. **Bad:** ```javascript -class Airplane { +function makeAirplane(config) { // ... - getCruisingAltitude() { - switch (this.type) { + function getCruisingAltitude() { + switch (config.type) { case '777': - return this.getMaxAltitude() - this.getPassengerCount(); + return getMaxAltitude() - getPassengerCount(); case 'Air Force One': - return this.getMaxAltitude(); + return getMaxAltitude(); case 'Cessna': - return this.getMaxAltitude() - this.getFuelExpenditure(); + return getMaxAltitude() - getFuelExpenditure(); } } + // ... } ``` **Good:** ```javascript -class Airplane { +function makeAirplane(config) { // ... } -class Boeing777 extends Airplane { - // ... - getCruisingAltitude() { - return this.getMaxAltitude() - this.getPassengerCount(); - } +function makeBoeing777(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude() - plane.getPassengerCount(); + + return plane; } -class AirForceOne extends Airplane { - // ... - getCruisingAltitude() { - return this.getMaxAltitude(); - } +function makeAirForceOne(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude(); + + return plane; } -class Cessna extends Airplane { - // ... - getCruisingAltitude() { - return this.getMaxAltitude() - this.getFuelExpenditure(); - } +function makeCessna(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude() - plane.getFuelExpenditure(); + + return plane; } ``` **[⬆ back to top](#table-of-contents)** From 360984454ca4cc26615f3c19dcd6ceec0af06191 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 14:19:00 +0100 Subject: [PATCH 23/52] avoid conditionals: example code --- examples/avoid-conditionals-BAD.js | 32 +++++++++++++++ examples/avoid-conditionals-GOOD.js | 61 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 examples/avoid-conditionals-BAD.js create mode 100644 examples/avoid-conditionals-GOOD.js diff --git a/examples/avoid-conditionals-BAD.js b/examples/avoid-conditionals-BAD.js new file mode 100644 index 00000000..179c6131 --- /dev/null +++ b/examples/avoid-conditionals-BAD.js @@ -0,0 +1,32 @@ +function makeAirplane(config) { + function getMaxAltitude() { + return config.altitude; +} + +function getPassengerCount() { + return config.passengers; +} + +function getFuelExpenditure() { + return config.fuelExpenditure; +} + + + function getCruisingAltitude() { + switch (config.type) { + case '777': + return getMaxAltitude() - getPassengerCount(); + case 'Air Force One': + return getMaxAltitude(); + case 'Cessna': + return getMaxAltitude() - getFuelExpenditure(); + } + } + + return { + getMaxAltitude, + getPassengerCount, + getFuelExpenditure, + getCruisingAltitude + } +} diff --git a/examples/avoid-conditionals-GOOD.js b/examples/avoid-conditionals-GOOD.js new file mode 100644 index 00000000..6faec17f --- /dev/null +++ b/examples/avoid-conditionals-GOOD.js @@ -0,0 +1,61 @@ +function makeAirplane(config) { + function getMaxAltitude() { + return config.altitude; + } + + function getPassengerCount() { + return config.passengers; + } + + function getFuelExpenditure() { + return config.fuelExpenditure; + } + + function getCruisingAltitude() { + return config.cruisingAltitude; + } + + return { + getMaxAltitude, + getPassengerCount, + getFuelExpenditure, + getCruisingAltitude + }; +} + +function makeBoeing777(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude() - plane.getPassengerCount(); + + return plane; +} + +function makeAirForceOne(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude(); + + return plane; +} + +function makeCessna(config) { + const plane = makeAirplane(config); + + plane.getCruisingAltitude = () => + plane.getMaxAltitude() - plane.getFuelExpenditure(); + + return plane; +} + +// example usage +const config = { + altitude: 1000, + passengers: 200, + fuelExpenditure: 10, + cruisingAltitude: 800 +} + +const cessna = makeCessna(config); From 8c472c143d9e47fa77078de3c0d42e43fb54b0be Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 14:22:33 +0100 Subject: [PATCH 24/52] to oneliners --- README.md | 9 +++------ examples/avoid-conditionals-GOOD.js | 9 +++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ba80e296..0f4e8e18 100644 --- a/README.md +++ b/README.md @@ -798,8 +798,7 @@ function makeAirplane(config) { function makeBoeing777(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude() - plane.getPassengerCount(); + plane.getCruisingAltitude = () => plane.getMaxAltitude() - plane.getPassengerCount(); return plane; } @@ -807,8 +806,7 @@ function makeBoeing777(config) { function makeAirForceOne(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude(); + plane.getCruisingAltitude = () => plane.getMaxAltitude(); return plane; } @@ -816,8 +814,7 @@ function makeAirForceOne(config) { function makeCessna(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude() - plane.getFuelExpenditure(); + plane.getCruisingAltitude = () => plane.getMaxAltitude() - plane.getFuelExpenditure(); return plane; } diff --git a/examples/avoid-conditionals-GOOD.js b/examples/avoid-conditionals-GOOD.js index 6faec17f..42484d12 100644 --- a/examples/avoid-conditionals-GOOD.js +++ b/examples/avoid-conditionals-GOOD.js @@ -26,8 +26,7 @@ function makeAirplane(config) { function makeBoeing777(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude() - plane.getPassengerCount(); + plane.getCruisingAltitude = () => plane.getMaxAltitude() - plane.getPassengerCount(); return plane; } @@ -35,8 +34,7 @@ function makeBoeing777(config) { function makeAirForceOne(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude(); + plane.getCruisingAltitude = () => plane.getMaxAltitude(); return plane; } @@ -44,8 +42,7 @@ function makeAirForceOne(config) { function makeCessna(config) { const plane = makeAirplane(config); - plane.getCruisingAltitude = () => - plane.getMaxAltitude() - plane.getFuelExpenditure(); + plane.getCruisingAltitude = () => plane.getMaxAltitude() - plane.getFuelExpenditure(); return plane; } From e623b726d39a21ea6fb6a861c20f39d4e8ebaf3e Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 14:26:04 +0100 Subject: [PATCH 25/52] link to full example --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 0f4e8e18..0d89a0c1 100644 --- a/README.md +++ b/README.md @@ -819,6 +819,8 @@ function makeCessna(config) { return plane; } ``` +[full example](examples/avoid-conditionals-GOOD.js) + **[⬆ back to top](#table-of-contents)** ### Avoid type-checking (part 1) From 0708849c4662242e03e159adbacd3ff63b142dd2 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 22:07:59 +0100 Subject: [PATCH 26/52] use getters and setters as functions, without the get and set keywords --- README.md | 60 ++++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 0d89a0c1..d2ef2b16 100644 --- a/README.md +++ b/README.md @@ -937,9 +937,7 @@ inventoryTracker('apples', req, 'www.inventory-awesome.io'); ## **Objects and Data Structures** ### Use getters and setters -JavaScript doesn't have interfaces or types so it is very hard to enforce this -pattern, because we don't have keywords like `public` and `private`. As it is, -using getters and setters to access data on objects is far better than simply +Using getters and setters to access data on objects could be better than simply looking for a property on an object. "Why?" you might ask. Well, here's an unorganized list of reasons why: @@ -948,56 +946,54 @@ to look up and change every accessor in your codebase. * Makes adding validation simple when doing a `set`. * Encapsulates the internal representation. * Easy to add logging and error handling when getting and setting. -* Inheriting this class, you can override default functionality. * You can lazy load your object's properties, let's say getting it from a server. **Bad:** ```javascript -class BankAccount { - constructor() { - this.balance = 1000; - } -} +function makeBankAccount() { + // ... -const bankAccount = new BankAccount(); + return { + balance: 0, + // ... + }; +} -// Buy shoes... -bankAccount.balance -= 100; +const account = makeBankAccount(); +account.balance = 100; ``` **Good:** ```javascript -class BankAccount { - constructor(balance = 1000) { - this._balance = balance; +function makeBankAccount() { + let balance = 0; + // ... + + function isValid(amount) { + // ... } - // It doesn't have to be prefixed with `get` or `set` to be a getter/setter - set balance(amount) { - if (this.verifyIfAmountCanBeSetted(amount)) { - this._balance = amount; - } + function getBalance() { + return balance; } - get balance() { - return this._balance; + function setBalance() { + if (isValid(amount)) { + balance = amount; + } } - verifyIfAmountCanBeSetted(val) { + return { // ... - } + getBalance, + setBalance + }; } -const bankAccount = new BankAccount(); - -// Buy shoes... -bankAccount.balance -= shoesPrice; - -// Get balance -let balance = bankAccount.balance; - +const account = makeBankAccount(); +account.setBalance(100); ``` **[⬆ back to top](#table-of-contents)** From 0731e30f79ae15a401a01b424aa3593f23a0215b Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 22:34:14 +0100 Subject: [PATCH 27/52] refactor: use getters and setters GOOD example code --- README.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index d2ef2b16..089cd95d 100644 --- a/README.md +++ b/README.md @@ -968,21 +968,18 @@ account.balance = 100; **Good:** ```javascript function makeBankAccount() { + // this one is private let balance = 0; - // ... - - function isValid(amount) { - // ... - } + // a "getter", made public via the returned object below function getBalance() { return balance; } - function setBalance() { - if (isValid(amount)) { - balance = amount; - } + // a "setter", made public via the returned object below + function setBalance(amount) { + // ... validate before updating the balance + balance = amount; } return { @@ -995,6 +992,8 @@ function makeBankAccount() { const account = makeBankAccount(); account.setBalance(100); ``` +[full example](examples/use-getters-and-setters-GOOD.js) + **[⬆ back to top](#table-of-contents)** From e6e50570662326a58e03ce7743c0d97fb3566a38 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 22:44:53 +0100 Subject: [PATCH 28/52] use getters and setters code examples --- examples/use-getters-and-setters-BAD.js | 11 +++++++++++ examples/use-getters-and-setters-GOOD.js | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 examples/use-getters-and-setters-BAD.js create mode 100644 examples/use-getters-and-setters-GOOD.js diff --git a/examples/use-getters-and-setters-BAD.js b/examples/use-getters-and-setters-BAD.js new file mode 100644 index 00000000..8a268dbf --- /dev/null +++ b/examples/use-getters-and-setters-BAD.js @@ -0,0 +1,11 @@ +function makeBankAccount() { + // ... + + return { + balance: 0, + // ... + }; +} + +const account = makeBankAccount(); +account.balance = 100; diff --git a/examples/use-getters-and-setters-GOOD.js b/examples/use-getters-and-setters-GOOD.js new file mode 100644 index 00000000..70fc9140 --- /dev/null +++ b/examples/use-getters-and-setters-GOOD.js @@ -0,0 +1,24 @@ +function makeBankAccount() { + // this one is private + let balance = 0; + + // a "getter", made public via the returned object below + function getBalance() { + return balance; + } + + // a "setter", made public via the returned object below + function setBalance(amount) { + // ... validate before updating the balance + balance = amount; + } + + return { + // ... + getBalance, + setBalance + }; +} + +const account = makeBankAccount(); +account.setBalance(100); From dd1d1c6a4a1213879088d988ef4e9a6645437f74 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 28 Jan 2017 22:51:22 +0100 Subject: [PATCH 29/52] srp section rewrite --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 089cd95d..dfd89a2e 100644 --- a/README.md +++ b/README.md @@ -1066,7 +1066,7 @@ class UserSettings { **Good:** ```javascript -// the module userAuth.js +// code in a module: userAuth.js function hasValidCredentials(user) { // ... } @@ -1074,7 +1074,9 @@ function hasValidCredentials(user) { export default hasValidCredentials; -// the module userSettings.js + + +// code in a different module: userSettings.js import hasValidCredentials from 'userAuth.js'; function changeSettings(user, settings) { @@ -1082,8 +1084,6 @@ function changeSettings(user, settings) { // ... } } - -export default changeSettings; ``` **[⬆ back to top](#table-of-contents)** From 4cbf963c5a43a8d230b0a799c445e9e3f29c88ed Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 21:16:01 +0100 Subject: [PATCH 30/52] fix: code example --- README.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index dfd89a2e..5d787557 100644 --- a/README.md +++ b/README.md @@ -1066,17 +1066,15 @@ class UserSettings { **Good:** ```javascript -// code in a module: userAuth.js function hasValidCredentials(user) { // ... } export default hasValidCredentials; +``` - - - -// code in a different module: userSettings.js +Code in a separate module: +```javascript import hasValidCredentials from 'userAuth.js'; function changeSettings(user, settings) { From 08af9dfafce008e9acf2f3959eeaf233fe81f9ae Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:06:48 +0100 Subject: [PATCH 31/52] fix: remove text --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 5d787557..4aa9ad9d 100644 --- a/README.md +++ b/README.md @@ -1073,7 +1073,6 @@ function hasValidCredentials(user) { export default hasValidCredentials; ``` -Code in a separate module: ```javascript import hasValidCredentials from 'userAuth.js'; From 19048892d9f5eda53302a5b34952d882c5dd8ba0 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:37:36 +0100 Subject: [PATCH 32/52] open-closed principle as functions, with full example code --- README.md | 45 ++++++++++++++------------ examples/open-closed-principle-GOOD.js | 43 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 examples/open-closed-principle-GOOD.js diff --git a/README.md b/README.md index 4aa9ad9d..812ec955 100644 --- a/README.md +++ b/README.md @@ -1135,40 +1135,45 @@ function makeHttpCall(url) { **Good:** ```javascript -class AjaxAdapter extends Adapter { - constructor() { - super(); - this.name = 'ajaxAdapter'; - } +function makeAjaxAdapter() { + let adapter = makeAdapter('ajaxAdapter'); - request(url) { - // request and return promise + adapter.request = function (url) { + // ... request, do ajax adapter work and return promise } + + return adapter; } -class NodeAdapter extends Adapter { - constructor() { - super(); - this.name = 'nodeAdapter'; - } +function makeNodeAdapter() { + let adapter = makeAdapter('nodeAdapter'); - request(url) { - // request and return promise + adapter.request = function (url) { + // ... request, do node adapter work and return promise } -} -class HttpRequester { - constructor(adapter) { - this.adapter = adapter; - } + return adapter; +} +function makeHttpRequester(adapter) { fetch(url) { - return this.adapter.request(url).then((response) => { + return adapter.request(url).then((response) => { // transform response and return }); } + + return { + fetch, + } } + +// example usage +const adapter = makeNodeAdapter(); +const requester = makeHttpRequester(adapter); +requester.fetch('http://github.com').then((res) => console.log(res)); ``` +[full example](examples(open-closed-principle-GOOD.js)) + **[⬆ back to top](#table-of-contents)** diff --git a/examples/open-closed-principle-GOOD.js b/examples/open-closed-principle-GOOD.js new file mode 100644 index 00000000..c7ec90f3 --- /dev/null +++ b/examples/open-closed-principle-GOOD.js @@ -0,0 +1,43 @@ +function makeAdapter(name) { + // .. + return { + name, + }; +} + +function makeAjaxAdapter() { + let adapter = makeAdapter('ajaxAdapter'); + + adapter.request = function (url) { + // ... request, do ajax adapter work and return promise + } + + return adapter; +} + +function makeNodeAdapter() { + let adapter = makeAdapter('nodeAdapter'); + + adapter.request = function (url) { + // ... request, do node adapter work and return promise + } + + return adapter; +} + +function makeHttpRequester(adapter) { + fetch(url) { + return adapter.request(url).then((response) => { + // transform response and return + }); + } + + return { + fetch, + } +} + +// example usage +const adapter = makeNodeAdapter(); +const requester = makeHttpRequester(adapter); +requester.fetch('http://github.com').then((res) => console.log(res)); From 0a5581f9ca5a27feb2b563827b421c3401a566a9 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:39:36 +0100 Subject: [PATCH 33/52] fix: removed example usage --- README.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/README.md b/README.md index 812ec955..ad6a42da 100644 --- a/README.md +++ b/README.md @@ -1166,11 +1166,6 @@ function makeHttpRequester(adapter) { fetch, } } - -// example usage -const adapter = makeNodeAdapter(); -const requester = makeHttpRequester(adapter); -requester.fetch('http://github.com').then((res) => console.log(res)); ``` [full example](examples(open-closed-principle-GOOD.js)) From 2fae9246f98ef45802b67c03c18b03015ddc7696 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:45:53 +0100 Subject: [PATCH 34/52] refactor: open-closed Good/Bad examples --- README.md | 30 +++++++++++++------------- examples/open-closed-principle-GOOD.js | 29 +++++++++++-------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index ad6a42da..6bd58a7a 100644 --- a/README.md +++ b/README.md @@ -1092,16 +1092,14 @@ add new functionalities without changing existing code. **Bad:** ```javascript -class AjaxAdapter extends Adapter { +class AjaxAdapter { constructor() { - super(); this.name = 'ajaxAdapter'; } } -class NodeAdapter extends Adapter { +class NodeAdapter { constructor() { - super(); this.name = 'nodeAdapter'; } } @@ -1116,7 +1114,7 @@ class HttpRequester { return makeAjaxCall(url).then((response) => { // transform response and return }); - } else if (this.adapter.name === 'httpNodeAdapter') { + } else if (this.adapter.name === 'nodeAdapter') { return makeHttpCall(url).then((response) => { // transform response and return }); @@ -1136,23 +1134,25 @@ function makeHttpCall(url) { **Good:** ```javascript function makeAjaxAdapter() { - let adapter = makeAdapter('ajaxAdapter'); - - adapter.request = function (url) { - // ... request, do ajax adapter work and return promise + // .. + function request(url) { + // ... request, do ajax work and return promise } - return adapter; + return { + request, + }; } function makeNodeAdapter() { - let adapter = makeAdapter('nodeAdapter'); - - adapter.request = function (url) { - // ... request, do node adapter work and return promise + // .. + function request(url) { + // ... request, do node work and return promise } - return adapter; + return { + request, + }; } function makeHttpRequester(adapter) { diff --git a/examples/open-closed-principle-GOOD.js b/examples/open-closed-principle-GOOD.js index c7ec90f3..654e43c8 100644 --- a/examples/open-closed-principle-GOOD.js +++ b/examples/open-closed-principle-GOOD.js @@ -1,28 +1,23 @@ -function makeAdapter(name) { - // .. - return { - name, - }; -} - function makeAjaxAdapter() { - let adapter = makeAdapter('ajaxAdapter'); - - adapter.request = function (url) { - // ... request, do ajax adapter work and return promise + // .. + function request(url) { + // ... request, do ajax work and return promise } - return adapter; + return { + request, + }; } function makeNodeAdapter() { - let adapter = makeAdapter('nodeAdapter'); - - adapter.request = function (url) { - // ... request, do node adapter work and return promise + // .. + function request(url) { + // ... request, do node work and return promise } - return adapter; + return { + request, + }; } function makeHttpRequester(adapter) { From f472f031e44e5bc0528237295b5ae9d4e2055d56 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:47:47 +0100 Subject: [PATCH 35/52] fix: typo in full example url --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6bd58a7a..be67ea2c 100644 --- a/README.md +++ b/README.md @@ -1167,7 +1167,7 @@ function makeHttpRequester(adapter) { } } ``` -[full example](examples(open-closed-principle-GOOD.js)) +[full example](examples/open-closed-principle-GOOD.js)) **[⬆ back to top](#table-of-contents)** From 87f0e9f31559fac73a8de9a7cca04315d0702141 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 22:48:17 +0100 Subject: [PATCH 36/52] fix: typo in full example url --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index be67ea2c..d3e33023 100644 --- a/README.md +++ b/README.md @@ -1167,7 +1167,7 @@ function makeHttpRequester(adapter) { } } ``` -[full example](examples/open-closed-principle-GOOD.js)) +[full example](examples/open-closed-principle-GOOD.js) **[⬆ back to top](#table-of-contents)** From 3d206784eda7ae11bcf437b07552803b3935a839 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sun, 29 Jan 2017 23:01:56 +0100 Subject: [PATCH 37/52] add open-closed Bad code example --- examples/open-closed-principle-BAD.js | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 examples/open-closed-principle-BAD.js diff --git a/examples/open-closed-principle-BAD.js b/examples/open-closed-principle-BAD.js new file mode 100644 index 00000000..7555cf92 --- /dev/null +++ b/examples/open-closed-principle-BAD.js @@ -0,0 +1,29 @@ +class AjaxAdapter { + constructor() { + this.name = 'ajaxAdapter'; + } +} + +class NodeAdapter { + constructor() { + this.name = 'nodeAdapter'; + } +} + +class HttpRequester { + constructor(adapter) { + this.adapter = adapter; + } + + fetch(url) { + if (this.adapter.name === 'ajaxAdapter') { + return makeAjaxCall(url).then((response) => { + // transform response and return + }); + } else if (this.adapter.name === 'nodeAdapter') { + return makeHttpCall(url).then((response) => { + // transform response and return + }); + } + } +} From 0cfa349ad7debc458111c0dfebb35d9bbe58ee5d Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 30 Jan 2017 12:48:17 +0100 Subject: [PATCH 38/52] liskov substitution Good example as functions --- README.md | 59 ++++++++-------------------- examples/liskov-substitution-GOOD.js | 38 ++++++++++++++++++ 2 files changed, 55 insertions(+), 42 deletions(-) create mode 100644 examples/liskov-substitution-GOOD.js diff --git a/README.md b/README.md index d3e33023..0136b671 100644 --- a/README.md +++ b/README.md @@ -1242,68 +1242,43 @@ renderLargeRectangles(rectangles); **Good:** ```javascript -class Shape { - setColor(color) { +function makeShape() { + function setColor(color) { // ... } - render(area) { + function render(area) { // ... } -} -class Rectangle extends Shape { - constructor() { - super(); - this.width = 0; - this.height = 0; - } - - setWidth(width) { - this.width = width; - } + return { + setColor, + render + }; +} - setHeight(height) { - this.height = height; - } +function makeRectangle(width, height) { + let shape = makeShape(); + shape.getArea = () => width * height; - getArea() { - return this.width * this.height; - } + return shape; } -class Square extends Shape { - constructor() { - super(); - this.length = 0; - } - - setLength(length) { - this.length = length; - } +function makeSquare(length) { + let shape = makeShape(); + shape.getArea = () => length * length; - getArea() { - return this.length * this.length; - } + return shape; } function renderLargeShapes(shapes) { shapes.forEach((shape) => { - switch (shape.constructor.name) { - case 'Square': - shape.setLength(5); - break; - case 'Rectangle': - shape.setWidth(4); - shape.setHeight(5); - } - const area = shape.getArea(); shape.render(area); }); } -const shapes = [new Rectangle(), new Rectangle(), new Square()]; +const shapes = [makeRectangle(3, 4), makeRectangle(4, 5), makeSquare(4)]; renderLargeShapes(shapes); ``` **[⬆ back to top](#table-of-contents)** diff --git a/examples/liskov-substitution-GOOD.js b/examples/liskov-substitution-GOOD.js new file mode 100644 index 00000000..8c606b53 --- /dev/null +++ b/examples/liskov-substitution-GOOD.js @@ -0,0 +1,38 @@ +function makeShape() { + function setColor(color) { + // ... + } + + function render(area) { + // ... + } + + return { + setColor, + render + }; +} + +function makeRectangle(width, height) { + let shape = makeShape(); + shape.getArea = () => width * height; + + return shape; +} + +function makeSquare(length) { + let shape = makeShape(); + shape.getArea = () => length * length; + + return shape; +} + +function renderLargeShapes(shapes) { + shapes.forEach((shape) => { + const area = shape.getArea(); + shape.render(area); + }); +} + +const shapes = [makeRectangle(3, 4), makeRectangle(4, 5), makeSquare(4)]; +renderLargeShapes(shapes); From 975d84310628b62ec56d847a7b8ab1fba2888943 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 30 Jan 2017 12:50:36 +0100 Subject: [PATCH 39/52] full example with log to verify code working --- examples/liskov-substitution-GOOD.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/liskov-substitution-GOOD.js b/examples/liskov-substitution-GOOD.js index 8c606b53..381a85ee 100644 --- a/examples/liskov-substitution-GOOD.js +++ b/examples/liskov-substitution-GOOD.js @@ -4,7 +4,7 @@ function makeShape() { } function render(area) { - // ... + console.log(area); } return { From a8d28da852b8af2a4c226222c0d41949e06640d1 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 30 Jan 2017 20:35:42 +0100 Subject: [PATCH 40/52] add editorconfig to avoid IDE/editor diffs --- .editorconfig | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..14869adf --- /dev/null +++ b/.editorconfig @@ -0,0 +1,10 @@ +root = true + +[*] +end_of_line = lf +insert_final_newline = true + +[*.{js,json,md}] +charset = utf-8 +indent_style = space +indent_size = 2 From d72f5fac7f8c7b49dab37829a833e2f0667c7d44 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 30 Jan 2017 20:58:28 +0100 Subject: [PATCH 41/52] fix: lint errors --- .eslintrc.json | 24 ++++++++++++++++++ .gitignore | 1 + README.md | 6 ++--- examples/avoid-conditionals-BAD.js | 19 +++++++------- examples/avoid-conditionals-GOOD.js | 2 +- examples/open-closed-principle-BAD.js | 1 + examples/open-closed-principle-GOOD.js | 5 ++-- examples/use-getters-and-setters-GOOD.js | 32 ++++++++++++------------ package.json | 25 ++++++++++++++++++ 9 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 .eslintrc.json create mode 100644 .gitignore create mode 100644 package.json diff --git a/.eslintrc.json b/.eslintrc.json new file mode 100644 index 00000000..9acea47c --- /dev/null +++ b/.eslintrc.json @@ -0,0 +1,24 @@ +{ + "parserOptions": { + "ecmaVersion": "2017", + "sourceType": "module" + }, + "rules": { + "eqeqeq": 2, + "no-console": 0, + "quotes": [2, "single"], + "linebreak-style": [2, "unix"], + "semi": [2, "always"], + "no-unused-vars": 0, + "react/prop-types": 0 + }, + "globals": { + "define": true + }, + "env": { + "es6": true, + "browser": true, + "commonjs": true + }, + "extends": ["eslint:recommended"] +} diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..c2658d7d --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +node_modules/ diff --git a/README.md b/README.md index 0136b671..c131e608 100644 --- a/README.md +++ b/README.md @@ -985,7 +985,7 @@ function makeBankAccount() { return { // ... getBalance, - setBalance + setBalance, }; } @@ -1156,7 +1156,7 @@ function makeNodeAdapter() { } function makeHttpRequester(adapter) { - fetch(url) { + function fetch(url) { return adapter.request(url).then((response) => { // transform response and return }); @@ -1164,7 +1164,7 @@ function makeHttpRequester(adapter) { return { fetch, - } + }; } ``` [full example](examples/open-closed-principle-GOOD.js) diff --git a/examples/avoid-conditionals-BAD.js b/examples/avoid-conditionals-BAD.js index 179c6131..cbb4ed77 100644 --- a/examples/avoid-conditionals-BAD.js +++ b/examples/avoid-conditionals-BAD.js @@ -1,16 +1,15 @@ function makeAirplane(config) { function getMaxAltitude() { - return config.altitude; -} - -function getPassengerCount() { - return config.passengers; -} + return config.altitude; + } -function getFuelExpenditure() { - return config.fuelExpenditure; -} + function getPassengerCount() { + return config.passengers; + } + function getFuelExpenditure() { + return config.fuelExpenditure; + } function getCruisingAltitude() { switch (config.type) { @@ -28,5 +27,5 @@ function getFuelExpenditure() { getPassengerCount, getFuelExpenditure, getCruisingAltitude - } + }; } diff --git a/examples/avoid-conditionals-GOOD.js b/examples/avoid-conditionals-GOOD.js index 42484d12..a7cc48e9 100644 --- a/examples/avoid-conditionals-GOOD.js +++ b/examples/avoid-conditionals-GOOD.js @@ -53,6 +53,6 @@ const config = { passengers: 200, fuelExpenditure: 10, cruisingAltitude: 800 -} +}; const cessna = makeCessna(config); diff --git a/examples/open-closed-principle-BAD.js b/examples/open-closed-principle-BAD.js index 7555cf92..c6f159b2 100644 --- a/examples/open-closed-principle-BAD.js +++ b/examples/open-closed-principle-BAD.js @@ -1,3 +1,4 @@ +/*global makeAjaxCall, makeHttpCall */ class AjaxAdapter { constructor() { this.name = 'ajaxAdapter'; diff --git a/examples/open-closed-principle-GOOD.js b/examples/open-closed-principle-GOOD.js index 654e43c8..ae087f50 100644 --- a/examples/open-closed-principle-GOOD.js +++ b/examples/open-closed-principle-GOOD.js @@ -1,3 +1,4 @@ +/*global fetch */ function makeAjaxAdapter() { // .. function request(url) { @@ -21,7 +22,7 @@ function makeNodeAdapter() { } function makeHttpRequester(adapter) { - fetch(url) { + function fetch(url) { return adapter.request(url).then((response) => { // transform response and return }); @@ -29,7 +30,7 @@ function makeHttpRequester(adapter) { return { fetch, - } + }; } // example usage diff --git a/examples/use-getters-and-setters-GOOD.js b/examples/use-getters-and-setters-GOOD.js index 70fc9140..55994faf 100644 --- a/examples/use-getters-and-setters-GOOD.js +++ b/examples/use-getters-and-setters-GOOD.js @@ -1,23 +1,23 @@ function makeBankAccount() { - // this one is private - let balance = 0; + // this one is private + let balance = 0; - // a "getter", made public via the returned object below - function getBalance() { - return balance; - } + // a "getter", made public via the returned object below + function getBalance() { + return balance; + } - // a "setter", made public via the returned object below - function setBalance(amount) { - // ... validate before updating the balance - balance = amount; - } + // a "setter", made public via the returned object below + function setBalance(amount) { + // ... validate before updating the balance + balance = amount; + } - return { - // ... - getBalance, - setBalance - }; + return { + // ... + getBalance, + setBalance, + }; } const account = makeBankAccount(); diff --git a/package.json b/package.json new file mode 100644 index 00000000..29be94e9 --- /dev/null +++ b/package.json @@ -0,0 +1,25 @@ +{ + "name": "clean-code-javascript-func-remix", + "version": "1.0.0", + "description": "Clean Code JavaScript (Davids func remix)", + "main": "index.js", + "directories": { + "example": "examples" + }, + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/DavidVujic/clean-code-javascript.git" + }, + "author": "David Vujic", + "license": "MIT", + "bugs": { + "url": "https://github.com/DavidVujic/clean-code-javascript/issues" + }, + "homepage": "https://github.com/DavidVujic/clean-code-javascript#readme", + "devDependencies": { + "eslint": "^3.14.1" + } +} From d242f1cb44e60dbbaef9135dc82d3f886e39d162 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Tue, 31 Jan 2017 09:32:06 +0100 Subject: [PATCH 42/52] simplify example --- README.md | 10 ++++------ examples/open-closed-principle-GOOD.js | 12 +++++------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index c131e608..ea410e40 100644 --- a/README.md +++ b/README.md @@ -1134,24 +1134,22 @@ function makeHttpCall(url) { **Good:** ```javascript function makeAjaxAdapter() { - // .. function request(url) { - // ... request, do ajax work and return promise + // ... make an http request, do AJAX work and return promise } return { - request, + request }; } function makeNodeAdapter() { - // .. function request(url) { - // ... request, do node work and return promise + // ... make an http request, do NODE work and return promise } return { - request, + request }; } diff --git a/examples/open-closed-principle-GOOD.js b/examples/open-closed-principle-GOOD.js index ae087f50..452c2e2c 100644 --- a/examples/open-closed-principle-GOOD.js +++ b/examples/open-closed-principle-GOOD.js @@ -1,23 +1,21 @@ /*global fetch */ function makeAjaxAdapter() { - // .. function request(url) { - // ... request, do ajax work and return promise + // ... make an http request, do AJAX work and return promise } return { - request, + request }; } function makeNodeAdapter() { - // .. function request(url) { - // ... request, do node work and return promise + // ... make an request, do NODE work and return promise } return { - request, + request }; } @@ -29,7 +27,7 @@ function makeHttpRequester(adapter) { } return { - fetch, + fetch }; } From e2de879f805a22a0aff656926244a2310841da0e Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 2 Feb 2017 23:05:33 +0100 Subject: [PATCH 43/52] WIP: interface segregation example --- .../interface-segregation-principle-GOOD.js | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 examples/interface-segregation-principle-GOOD.js diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js new file mode 100644 index 00000000..830d3c45 --- /dev/null +++ b/examples/interface-segregation-principle-GOOD.js @@ -0,0 +1,56 @@ +function makeMessage(obj) { + function send() { + console.log(obj); + } + return { + send + }; +} + +function makeRating(obj) { + obj.stars = 0; + + function rate(stars) { + obj.stars = stars; + } + + return { + rate + }; +} + +function makeFeedback(obj) { + obj.feedback = []; + + function share(message) { + obj.feedback.push(message); + } + + return { + share + }; +} + +function makeFeedbackMessage() { + var obj = {}; + var message = makeMessage(obj); + var feedback = makeFeedback(obj); + + return { + share: feedback.share, + send: message.send + }; +} + +function makeUserMessage() { + var obj = {}; + var message = makeMessage(obj); + var feedback = makeFeedback(obj); + var rating = makeRating(obj); + + return { + share: feedback.share, + rate: rating.rate, + send: message.send + }; +} From 98f41edcd24701bf52ca5478d379cc79fc8fec0b Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 2 Feb 2017 23:08:13 +0100 Subject: [PATCH 44/52] WIP: interface segregation example --- examples/interface-segregation-principle-GOOD.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index 830d3c45..c17a6a2a 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -31,17 +31,6 @@ function makeFeedback(obj) { }; } -function makeFeedbackMessage() { - var obj = {}; - var message = makeMessage(obj); - var feedback = makeFeedback(obj); - - return { - share: feedback.share, - send: message.send - }; -} - function makeUserMessage() { var obj = {}; var message = makeMessage(obj); From 1980e3e0be761fc8c7422788e6e5953bf8dedbab Mon Sep 17 00:00:00 2001 From: davidvujic Date: Thu, 2 Feb 2017 23:15:55 +0100 Subject: [PATCH 45/52] WIP: two implementations --- examples/interface-segregation-principle-GOOD.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index c17a6a2a..91ab5d51 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -31,14 +31,25 @@ function makeFeedback(obj) { }; } -function makeUserMessage() { +// Implementations + +function makeFeedbackMessage() { var obj = {}; var message = makeMessage(obj); var feedback = makeFeedback(obj); - var rating = makeRating(obj); return { share: feedback.share, + send: message.send + }; +} + +function makeRatingMessage() { + var obj = {}; + var message = makeMessage(obj); + var rating = makeRating(obj); + + return { rate: rating.rate, send: message.send }; From 8b348986a99c31f480fb34f5beb9b37d04d0cebd Mon Sep 17 00:00:00 2001 From: davidvujic Date: Fri, 3 Feb 2017 08:54:50 +0100 Subject: [PATCH 46/52] WIP: interface segregation principle, without passing an object --- .../interface-segregation-principle-GOOD.js | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index 91ab5d51..c50e5ee2 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -1,56 +1,65 @@ -function makeMessage(obj) { - function send() { - console.log(obj); - } - return { - send - }; +function sendMessage(message) { + console.log(message); } + function makeRating(obj) { - obj.stars = 0; + let rating = 0; - function rate(stars) { - obj.stars = stars; - } + const get = () => rating; + const set = (stars) => rating = stars; return { - rate + get, + set }; } -function makeFeedback(obj) { - obj.feedback = []; +function makeFeedback() { + const feedback = []; - function share(message) { - obj.feedback.push(message); - } + const get = () => feedback; + const add = (message) => feedback.push(message); return { - share + get, + add }; } // Implementations function makeFeedbackMessage() { - var obj = {}; - var message = makeMessage(obj); - var feedback = makeFeedback(obj); + var feedback = makeFeedback(); + + function share(message) { + feedback.add(message); + } + + function send() { + sendMessage(feedback.get()); + } return { - share: feedback.share, - send: message.send + share, + send }; } + function makeRatingMessage() { - var obj = {}; - var message = makeMessage(obj); - var rating = makeRating(obj); + var ratings = makeRating(); + + function rate(stars) { + ratings.set(stars); + } + + function send() { + sendMessage(ratings.get()); + } return { - rate: rating.rate, - send: message.send + rate, + send }; } From 799bda9e12323ff549ee47486a9e288b629b0b13 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 4 Feb 2017 13:19:17 +0100 Subject: [PATCH 47/52] refactor: interface segregation example code --- .../interface-segregation-principle-GOOD.js | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index c50e5ee2..3277fa87 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -1,13 +1,13 @@ -function sendMessage(message) { +// "interfaces" +function postMessage(message) { console.log(message); } +function makeRating() { + let stars = 0; -function makeRating(obj) { - let rating = 0; - - const get = () => rating; - const set = (stars) => rating = stars; + const get = () => stars; + const set = (numberOfStars) => stars = numberOfStars; return { get, @@ -16,10 +16,10 @@ function makeRating(obj) { } function makeFeedback() { - const feedback = []; + const messages = []; - const get = () => feedback; - const add = (message) => feedback.push(message); + const get = () => messages; + const add = (message) => messages.push(message); return { get, @@ -27,39 +27,36 @@ function makeFeedback() { }; } -// Implementations +// "clients" -function makeFeedbackMessage() { +function messageForFeedback() { var feedback = makeFeedback(); - function share(message) { - feedback.add(message); - } - - function send() { - sendMessage(feedback.get()); - } + const share = (message) => feedback.add(message); return { share, - send + get: feedback.get }; } - -function makeRatingMessage() { +function messageForRating() { var ratings = makeRating(); - function rate(stars) { - ratings.set(stars); - } - - function send() { - sendMessage(ratings.get()); - } + const rate = (stars) => ratings.set(stars); + const send = () => postMessage(ratings.get()); return { rate, send }; } + +// usage +const feedback = messageForFeedback(); +feedback.share('Good job!'); +const results = feedback.get(); + +const ratings = messageForRating(); +ratings.rate(5); +ratings.send(); From 2b6f138a8e9d9a07deff443f92d2ff3228fbbb5a Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 4 Feb 2017 14:52:59 +0100 Subject: [PATCH 48/52] ISP more examples --- ...segregation-principle-class-version-BAD.js | 38 ++++++++++ ...egregation-principle-class-version-GOOD.js | 69 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 examples/interface-segregation-principle-class-version-BAD.js create mode 100644 examples/interface-segregation-principle-class-version-GOOD.js diff --git a/examples/interface-segregation-principle-class-version-BAD.js b/examples/interface-segregation-principle-class-version-BAD.js new file mode 100644 index 00000000..01c425b1 --- /dev/null +++ b/examples/interface-segregation-principle-class-version-BAD.js @@ -0,0 +1,38 @@ +class Message { + constructor() { + this._feedback = []; + this._rating = 0; + } + + setRating(stars) { + this._rating = stars; + } + + addFeedback(message) { + this._feedback.push(message); + } + + getFeedback() { + return this._feedback; + } + + getRating() { + return this._rating; + } + + postMessage(message) { + console.log(message); + } +} + +// "clients" +class MessageForFeedback extends Message { + + share(message) { + return super.addFeedback(message); + } + + get() { + return super.getFeedback(); + } +} diff --git a/examples/interface-segregation-principle-class-version-GOOD.js b/examples/interface-segregation-principle-class-version-GOOD.js new file mode 100644 index 00000000..624200d2 --- /dev/null +++ b/examples/interface-segregation-principle-class-version-GOOD.js @@ -0,0 +1,69 @@ +// "interfaces" +function postMessage(message) { + console.log(message); +} + +class Rating { + constructor() { + this._rating = 0; + } + + get() { + return this._rating; + } + + set(stars) { + this._rating = stars; + } +} + +class Feedback { + constructor() { + this._feedback = []; + } + + get() { + return this._feedback; + } + + add(message) { + this._feedback.push(message); + } +} +// "clients" +class MessageForFeedback { + constructor() { + this._feedback = new Feedback(); + } + + share(message) { + return this._feedback.add(message); + } + + get() { + return this._feedback.get(); + } +} + +class MessageForRating { + constructor() { + this._ratings = new Rating(); + } + + rate(stars) { + return this._ratings.set(stars); + } + + send() { + postMessage(this._ratings.get()); + } +} + +// example usage +const feedback = new MessageForFeedback(); +feedback.share('Good job!'); +const result = feedback.get(); + +const ratings = new MessageForRating(); +ratings.rate(5); +ratings.send(); From fc1559d688ef309ecfd5e7430b62ad35d6a4235a Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 4 Feb 2017 14:54:20 +0100 Subject: [PATCH 49/52] ISP more examples --- ...rface-segregation-principle-class-version-BAD.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/examples/interface-segregation-principle-class-version-BAD.js b/examples/interface-segregation-principle-class-version-BAD.js index 01c425b1..1ee7b050 100644 --- a/examples/interface-segregation-principle-class-version-BAD.js +++ b/examples/interface-segregation-principle-class-version-BAD.js @@ -4,10 +4,6 @@ class Message { this._rating = 0; } - setRating(stars) { - this._rating = stars; - } - addFeedback(message) { this._feedback.push(message); } @@ -16,18 +12,23 @@ class Message { return this._feedback; } + // this method makes no sense to the Feedback subclass getRating() { return this._rating; } + // this method makes no sense to the Feedback subclass + setRating(stars) { + this._rating = stars; + } + + // this method makes no sense to the Feedback subclass postMessage(message) { console.log(message); } } -// "clients" class MessageForFeedback extends Message { - share(message) { return super.addFeedback(message); } From cf0565c76ff8f552567f6a96c7e9c98a81a184d9 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Sat, 4 Feb 2017 14:58:46 +0100 Subject: [PATCH 50/52] ISP Bad/Good examples refactored --- README.md | 142 ++++++++++++------ .../interface-segregation-principle-GOOD.js | 7 +- 2 files changed, 95 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index ea410e40..406dc032 100644 --- a/README.md +++ b/README.md @@ -1282,76 +1282,118 @@ renderLargeShapes(shapes); **[⬆ back to top](#table-of-contents)** ### Interface Segregation Principle (ISP) -JavaScript doesn't have interfaces so this principle doesn't apply as strictly -as others. However, it's important and relevant even with JavaScript's lack of -type system. - -ISP states that "Clients should not be forced to depend upon interfaces that -they do not use." Interfaces are implicit contracts in JavaScript because of -duck typing. - -A good example to look at that demonstrates this principle in JavaScript is for -classes that require large settings objects. Not requiring clients to setup -huge amounts of options is beneficial, because most of the time they won't need -all of the settings. Making them optional helps prevent having a "fat interface". +ISP states that "___Clients should not be forced to depend upon interfaces that +they do not use.___" In JavaScript, an interface is the contract of an object (i.e. the public properties and methods). In the "Bad" example below, the `Feedback` class inherits everything from the `Message` class, but only a couple of the features will +make sense to the subclass. **Bad:** ```javascript -class DOMTraverser { - constructor(settings) { - this.settings = settings; - this.setup(); +class Message { + constructor() { + this._feedback = []; + this._rating = 0; } - setup() { - this.rootNode = this.settings.rootNode; - this.animationModule.setup(); + addFeedback(message) { + this._feedback.push(message); } - traverse() { - // ... + getFeedback() { + return this._feedback; + } + + // this method makes no sense to the Feedback subclass + getRating() { + return this._rating; + } + + // this method makes no sense to the Feedback subclass + setRating(stars) { + this._rating = stars; + } + + // this method makes no sense to the Feedback subclass + postMessage(message) { + console.log(message); } } -const $ = new DOMTraverser({ - rootNode: document.getElementsByTagName('body'), - animationModule() {} // Most of the time, we won't need to animate when traversing. - // ... -}); +class MessageForFeedback extends Message { + share(message) { + return super.addFeedback(message); + } + get() { + return super.getFeedback(); + } +} ``` **Good:** ```javascript -class DOMTraverser { - constructor(settings) { - this.settings = settings; - this.options = settings.options; - this.setup(); - } +// several "interfaces", separated by feature +function postMessage(message) { + console.log(message); +} - setup() { - this.rootNode = this.settings.rootNode; - this.setupOptions(); - } +function makeRating() { + let stars = 0; - setupOptions() { - if (this.options.animationModule) { - // ... - } - } + const get = () => stars; + const set = (numberOfStars) => stars = numberOfStars; - traverse() { - // ... - } + return { + get, + set + }; } -const $ = new DOMTraverser({ - rootNode: document.getElementsByTagName('body'), - options: { - animationModule() {} - } -}); +function makeFeedback() { + const messages = []; + + const get = () => messages; + const add = (message) => messages.push(message); + + return { + get, + add + }; +} +``` + +```javascript +// different "clients", composed with features that makes sense +function messageForFeedback() { + var feedback = makeFeedback(); + + const share = (message) => feedback.add(message); + + return { + share, + get: feedback.get + }; +} + +function messageForRating() { + var ratings = makeRating(); + + const rate = (stars) => ratings.set(stars); + const send = () => postMessage(ratings.get()); + + return { + rate, + send + }; +} + +// example usage +const feedback = messageForFeedback(); +feedback.share('Good job!'); +const results = feedback.get(); + +const ratings = messageForRating(); +ratings.rate(5); +ratings.send(); ``` **[⬆ back to top](#table-of-contents)** diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index 3277fa87..9876651a 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -1,4 +1,4 @@ -// "interfaces" +// several "interfaces", separated by feature function postMessage(message) { console.log(message); } @@ -27,8 +27,7 @@ function makeFeedback() { }; } -// "clients" - +// different "clients", composed with features that makes sense function messageForFeedback() { var feedback = makeFeedback(); @@ -52,7 +51,7 @@ function messageForRating() { }; } -// usage +// example usage const feedback = messageForFeedback(); feedback.share('Good job!'); const results = feedback.get(); From 03809bb1df35db13f6d7ff112a2ffc9a0122c20f Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 6 Feb 2017 09:00:32 +0100 Subject: [PATCH 51/52] refactor: simplify ISP example --- README.md | 37 ++++--------------- .../interface-segregation-principle-GOOD.js | 37 ++++--------------- 2 files changed, 14 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 406dc032..de7ea103 100644 --- a/README.md +++ b/README.md @@ -1362,38 +1362,15 @@ function makeFeedback() { ``` ```javascript -// different "clients", composed with features that makes sense -function messageForFeedback() { - var feedback = makeFeedback(); +// example usage: different "clients", composed with features that makes sense +const feedback = makeFeedback(); +feedback.add('Good job!'); - const share = (message) => feedback.add(message); +const ratings = makeRating(); +ratings.set(5); - return { - share, - get: feedback.get - }; -} - -function messageForRating() { - var ratings = makeRating(); - - const rate = (stars) => ratings.set(stars); - const send = () => postMessage(ratings.get()); - - return { - rate, - send - }; -} - -// example usage -const feedback = messageForFeedback(); -feedback.share('Good job!'); -const results = feedback.get(); - -const ratings = messageForRating(); -ratings.rate(5); -ratings.send(); +postMessage(feedback); +postMessage(ratings); ``` **[⬆ back to top](#table-of-contents)** diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index 9876651a..c67e5b8c 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -27,35 +27,12 @@ function makeFeedback() { }; } -// different "clients", composed with features that makes sense -function messageForFeedback() { - var feedback = makeFeedback(); +// example usage: different "clients", composed with features that makes sense +const feedback = makeFeedback(); +feedback.add('Good job!'); - const share = (message) => feedback.add(message); +const ratings = makeRating(); +ratings.set(5); - return { - share, - get: feedback.get - }; -} - -function messageForRating() { - var ratings = makeRating(); - - const rate = (stars) => ratings.set(stars); - const send = () => postMessage(ratings.get()); - - return { - rate, - send - }; -} - -// example usage -const feedback = messageForFeedback(); -feedback.share('Good job!'); -const results = feedback.get(); - -const ratings = messageForRating(); -ratings.rate(5); -ratings.send(); +postMessage(feedback); +postMessage(ratings); From 1a31d2c8290eef1c531072a7a0642a14c51db956 Mon Sep 17 00:00:00 2001 From: davidvujic Date: Mon, 6 Feb 2017 09:03:05 +0100 Subject: [PATCH 52/52] fix: typo --- README.md | 4 ++-- examples/interface-segregation-principle-GOOD.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index de7ea103..5dea1fbe 100644 --- a/README.md +++ b/README.md @@ -1369,8 +1369,8 @@ feedback.add('Good job!'); const ratings = makeRating(); ratings.set(5); -postMessage(feedback); -postMessage(ratings); +postMessage(feedback.get()); +postMessage(ratings.get()); ``` **[⬆ back to top](#table-of-contents)** diff --git a/examples/interface-segregation-principle-GOOD.js b/examples/interface-segregation-principle-GOOD.js index c67e5b8c..fcec5ca0 100644 --- a/examples/interface-segregation-principle-GOOD.js +++ b/examples/interface-segregation-principle-GOOD.js @@ -34,5 +34,5 @@ feedback.add('Good job!'); const ratings = makeRating(); ratings.set(5); -postMessage(feedback); -postMessage(ratings); +postMessage(feedback.get()); +postMessage(ratings.get());