ℹ️ This repository is part of my Refactoring catalog based on Fowler's book with the same title. Please see kaiosilveira/refactoring for more details.
Before | After |
---|---|
result.push(`<p>title: ${person.photo.title}</p>`);
result.concat(photoData(person.photo));
function photoData(aPhoto) {
return [`<p>location: ${aPhoto.location}</p>`, `<p>date: ${aPhoto.data.toDateString()}</p>`];
} |
result.concat(photoData(person.photo));
function photoData(aPhoto) {
return [
`<p>title: ${aPhoto.title}</p>`,
`<p>location: ${location.title}</p>`,
`<p>date: ${location.date.toDateString()}</p>`,
];
} |
Inverse of: Move Statements to Callers
Duplication is one of the easiest code smells to detect and at the same time one of the most difficult ones to remove. This refactoring helps with cases where we know we can encapsulate a sequence of statements into an isolate function, but we can't do it straightaway without breaking callers.
Our working example is a somewhat simple markup generation program, where we render data of a photo that belongs to a person. This rendering also contains some metadata about the photo, such as location, caption and date.
The test suite is as straightforward as the code and covers the rendering process, making sure that the correct markup is being generated by the code.
describe('renderPerson', () => {
it('should create markup for a person', () => {
const markup = renderPerson({}, person);
expect(markup).toEqual(
[
'<p>Kaio</p>',
'<p>🌃</p>',
'<p>title: Me</p>',
'<p>location: Paris</p>',
'<p>date: Wed Jan 01 2020</p>',
].join('\n')
);
});
});
describe('photoDiv', () => {
it('should create markup for a photo', () => {
expect(photoDiv(photo)).toEqual(
[
'<div>',
'<p>title: Me</p>',
'<p>location: Paris</p>',
'<p>date: Wed Jan 01 2020</p>',
'</div>',
].join('\n')
);
});
});
Our main goal in this exercise is to merge the title
paragraph and the contents in emitPhotoData
into a single function. We start by extracting a function to generate the photo data, relying on emitPhotoData
:
diff --git a/src/index.js b/src/index.js
@@ -21,3 +21,7 @@
function emitPhotoData(aPhoto) {
result.push(`<p>date: ${aPhoto.date.toDateString()}</p>`);
return result.join('\n');
}
+
+function zznew(p) {
+ return [`<p>title: ${p.title}</p>`, emitPhotoData(p)].join('\n');
+}
Then, we can start updating callers. The first one is photoDiv
:
diff --git a/src/index.js b/src/index.js
@@ -8,7 +8,7 @@
export function renderPerson(_outStream, aPerson) {
}
export function photoDiv(p) {
- return ['<div>', `<p>title: ${p.title}</p>`, emitPhotoData(p), '</div>'].join('\n');
+ return ['<div>', zznew(p), '</div>'].join('\n');
}
function renderPhoto(_aPhoto) {
And the second one is renderPerson
:
diff --git a/src/index.js b/src/index.js
@@ -2,8 +2,7 @@
export function renderPerson(_outStream, aPerson) {
const result = [];
result.push(`<p>${aPerson.name}</p>`);
result.push(renderPhoto(aPerson.photo));
- result.push(`<p>title: ${aPerson.photo.title}</p>`);
- result.push(emitPhotoData(aPerson.photo));
+ result.push(zznew(aPerson.photo));
return result.join('\n');
}
Now that all callers are updated, we can safely inline emitPhotoData
into zznew
:
diff --git a/src/index.js b/src/index.js
@@ -22,5 +22,9 @@
function emitPhotoData(aPhoto) {
}
function zznew(p) {
- return [`<p>title: ${p.title}</p>`, emitPhotoData(p)].join('\n');
+ return [
+ `<p>title: ${p.title}</p>`,
+ `<p>location: ${p.location}</p>`,
+ `<p>date: ${p.date.toDateString()}</p>`,
+ ].join('\n');
}
And remove emitPhotoData
:
diff --git a/src/index.js b/src/index.js
@@ -14,13 +14,6 @@
function renderPhoto(_aPhoto) {
return `<p>🌃</p>`;
}
-function emitPhotoData(aPhoto) {
- const result = [];
- result.push(`<p>location: ${aPhoto.location}</p>`);
- result.push(`<p>date: ${aPhoto.date.toDateString()}</p>`);
- return result.join('\n');
-}
-
function zznew(p) {
return [
`<p>title: ${p.title}</p>`,
In a plot twist, though, we can rename zznew
to emitPhotoData
:
diff --git a/src/index.js b/src/index.js
export function renderPerson(_outStream, aPerson) {
const result = [];
result.push(`<p>${aPerson.name}</p>`);
result.push(renderPhoto(aPerson.photo));
- result.push(zznew(aPerson.photo));
+ result.push(emitPhotoData(aPerson.photo));
return result.join('\n');
}
export function photoDiv(p) {
- return ['<div>', zznew(p), '</div>'].join('\n');
+ return ['<div>', emitPhotoData(p), '</div>'].join('\n');
}
function renderPhoto(_aPhoto) {
return `<p>🌃</p>`;
}
-function zznew(p) {
+function emitPhotoData(aPhoto) {
return [
- `<p>title: ${p.title}</p>`,
- `<p>location: ${p.location}</p>`,
- `<p>date: ${p.date.toDateString()}</p>`,
+ `<p>title: ${aPhoto.title}</p>`,
+ `<p>location: ${aPhoto.location}</p>`,
+ `<p>date: ${aPhoto.date.toDateString()}</p>`,
].join('\n');
}
And we're done!
Below there's the commit history for the steps detailed above.
Commit SHA | Message |
---|---|
eb2f48e | extract function to generate photo data |
6233389 | update photoDiv to use encapsulated version of photo data generation |
0834afa | update renderPerson to use new photo data function |
0b5b85a | inline emitPhotoData function into new photo generation function |
c99bc1e | remove unused emitPhotoData |
06e8eb3 | rename zznew to emitPhotoData |
For the full commit history for this project, check the Commit History tab.