Skip to content

Commit cf0565c

Browse files
committed
ISP Bad/Good examples refactored
1 parent fc1559d commit cf0565c

File tree

2 files changed

+95
-54
lines changed

2 files changed

+95
-54
lines changed

README.md

Lines changed: 92 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,76 +1282,118 @@ renderLargeShapes(shapes);
12821282
**[⬆ back to top](#table-of-contents)**
12831283

12841284
### Interface Segregation Principle (ISP)
1285-
JavaScript doesn't have interfaces so this principle doesn't apply as strictly
1286-
as others. However, it's important and relevant even with JavaScript's lack of
1287-
type system.
1288-
1289-
ISP states that "Clients should not be forced to depend upon interfaces that
1290-
they do not use." Interfaces are implicit contracts in JavaScript because of
1291-
duck typing.
1292-
1293-
A good example to look at that demonstrates this principle in JavaScript is for
1294-
classes that require large settings objects. Not requiring clients to setup
1295-
huge amounts of options is beneficial, because most of the time they won't need
1296-
all of the settings. Making them optional helps prevent having a "fat interface".
1285+
ISP states that "___Clients should not be forced to depend upon interfaces that
1286+
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
1287+
make sense to the subclass.
12971288

12981289
**Bad:**
12991290
```javascript
1300-
class DOMTraverser {
1301-
constructor(settings) {
1302-
this.settings = settings;
1303-
this.setup();
1291+
class Message {
1292+
constructor() {
1293+
this._feedback = [];
1294+
this._rating = 0;
13041295
}
13051296

1306-
setup() {
1307-
this.rootNode = this.settings.rootNode;
1308-
this.animationModule.setup();
1297+
addFeedback(message) {
1298+
this._feedback.push(message);
13091299
}
13101300

1311-
traverse() {
1312-
// ...
1301+
getFeedback() {
1302+
return this._feedback;
1303+
}
1304+
1305+
// this method makes no sense to the Feedback subclass
1306+
getRating() {
1307+
return this._rating;
1308+
}
1309+
1310+
// this method makes no sense to the Feedback subclass
1311+
setRating(stars) {
1312+
this._rating = stars;
1313+
}
1314+
1315+
// this method makes no sense to the Feedback subclass
1316+
postMessage(message) {
1317+
console.log(message);
13131318
}
13141319
}
13151320

1316-
const $ = new DOMTraverser({
1317-
rootNode: document.getElementsByTagName('body'),
1318-
animationModule() {} // Most of the time, we won't need to animate when traversing.
1319-
// ...
1320-
});
1321+
class MessageForFeedback extends Message {
1322+
share(message) {
1323+
return super.addFeedback(message);
1324+
}
13211325

1326+
get() {
1327+
return super.getFeedback();
1328+
}
1329+
}
13221330
```
13231331

13241332
**Good:**
13251333
```javascript
1326-
class DOMTraverser {
1327-
constructor(settings) {
1328-
this.settings = settings;
1329-
this.options = settings.options;
1330-
this.setup();
1331-
}
1334+
// several "interfaces", separated by feature
1335+
function postMessage(message) {
1336+
console.log(message);
1337+
}
13321338

1333-
setup() {
1334-
this.rootNode = this.settings.rootNode;
1335-
this.setupOptions();
1336-
}
1339+
function makeRating() {
1340+
let stars = 0;
13371341

1338-
setupOptions() {
1339-
if (this.options.animationModule) {
1340-
// ...
1341-
}
1342-
}
1342+
const get = () => stars;
1343+
const set = (numberOfStars) => stars = numberOfStars;
13431344

1344-
traverse() {
1345-
// ...
1346-
}
1345+
return {
1346+
get,
1347+
set
1348+
};
13471349
}
13481350

1349-
const $ = new DOMTraverser({
1350-
rootNode: document.getElementsByTagName('body'),
1351-
options: {
1352-
animationModule() {}
1353-
}
1354-
});
1351+
function makeFeedback() {
1352+
const messages = [];
1353+
1354+
const get = () => messages;
1355+
const add = (message) => messages.push(message);
1356+
1357+
return {
1358+
get,
1359+
add
1360+
};
1361+
}
1362+
```
1363+
1364+
```javascript
1365+
// different "clients", composed with features that makes sense
1366+
function messageForFeedback() {
1367+
var feedback = makeFeedback();
1368+
1369+
const share = (message) => feedback.add(message);
1370+
1371+
return {
1372+
share,
1373+
get: feedback.get
1374+
};
1375+
}
1376+
1377+
function messageForRating() {
1378+
var ratings = makeRating();
1379+
1380+
const rate = (stars) => ratings.set(stars);
1381+
const send = () => postMessage(ratings.get());
1382+
1383+
return {
1384+
rate,
1385+
send
1386+
};
1387+
}
1388+
1389+
// example usage
1390+
const feedback = messageForFeedback();
1391+
feedback.share('Good job!');
1392+
const results = feedback.get();
1393+
1394+
const ratings = messageForRating();
1395+
ratings.rate(5);
1396+
ratings.send();
13551397
```
13561398
**[⬆ back to top](#table-of-contents)**
13571399

examples/interface-segregation-principle-GOOD.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// "interfaces"
1+
// several "interfaces", separated by feature
22
function postMessage(message) {
33
console.log(message);
44
}
@@ -27,8 +27,7 @@ function makeFeedback() {
2727
};
2828
}
2929

30-
// "clients"
31-
30+
// different "clients", composed with features that makes sense
3231
function messageForFeedback() {
3332
var feedback = makeFeedback();
3433

@@ -52,7 +51,7 @@ function messageForRating() {
5251
};
5352
}
5453

55-
// usage
54+
// example usage
5655
const feedback = messageForFeedback();
5756
feedback.share('Good job!');
5857
const results = feedback.get();

0 commit comments

Comments
 (0)