The book is followed by the text.
24 Common Bad Smells and Refactoring in Code (Part 1)
Primitive Obsession
class Product {
constructor(data) {
this._name = data.name;
this._price = data.price;
/ *... * /
}
get name() {
return this.name;
}
/ *... * /
get price() {
return `The ${this.priceCount} The ${this.priceSuffix}`;
}
get priceCount() {
return parseFloat(this._price.slice(1));
}
get priceUnit() {
switch (this._price.slice(0.1)) {
case A '$':
return 'cny';
case '$':
return 'usd';
case 'k':
return 'hkd';
default:
throw new Error('un support unit'); }}get priceCnyCount() {
switch (this.priceUnit) {
case 'cny':
return this.priceCount;
case 'usd':
return this.priceCount * 7;
case 'hkd':
return this.priceCount * 0.8;
default:
throw new Error('un support unit'); }}get priceSuffix() {
switch (this.priceUnit) {
case 'cny':
return '元';
case 'usd':
return '$';
case 'hkd':
return 'Hong Kong';
default:
throw new Error('un support unit'); }}}Copy the code“Let’s take a look at the Product class. You can see some of the bad stuff in this class. The price field is a basic type, evaluated by various conversions in the Product class, and then output in different formats.
“In this case, price is well worth creating its own basic type — price.”
“Complete test case coverage before refactoring.” Geng started writing code.
describe('test Product price'.() = > {
const products = [
{ name: 'apple'.price: '$6' },
{ name: 'banana'.price: 'RMB 7' },
{ name: 'orange'.price: 'k15' },
{ name: 'cookie'.price: '$0.5'}]; test('Product.price should return correct price when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price);
expect(result).toStrictEqual(['$6'.'7 yuan'.'hk $15'.'$0.5']);
});
test('Product.price should return correct priceCount when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).priceCount);
expect(result).toStrictEqual([6.7.15.0.5]);
});
test('Product.price should return correct priceUnit when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).priceUnit);
expect(result).toStrictEqual(['usd'.'cny'.'hkd'.'usd']);
});
test('Product.price should return correct priceCnyCount when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).priceCnyCount);
expect(result).toStrictEqual([42.7.12.3.5]);
});
test('Product.price should return correct priceSuffix when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).priceSuffix);
expect(result).toStrictEqual(['$'.'元'.'Hong Kong'.'$']);
});
});
Copy the code“Run the test case after you’ve written it and see what happens.”
“It’s easy to create a new Price class, move the Price and related behavior to the Price class, and delegate it to the Product class. Let’s implement the Price class first.”
class Price {
constructor(value) {
this._value = value;
}
toString() {
return `The ${this.count} The ${this.suffix}`;
}
get count() {
return parseFloat(this._value.slice(1));
}
get unit() {
switch (this._value.slice(0.1)) {
case A '$':
return 'cny';
case '$':
return 'usd';
case 'k':
return 'hkd';
default:
throw new Error('un support unit'); }}get cnyCount() {
switch (this.unit) {
case 'cny':
return this.count;
case 'usd':
return this.count * 7;
case 'hkd':
return this.count * 0.8;
default:
throw new Error('un support unit'); }}get suffix() {
switch (this.unit) {
case 'cny':
return '元';
case 'usd':
return '$';
case 'hkd':
return 'Hong Kong';
default:
throw new Error('un support unit'); }}}Copy the code“At this point, I haven’t modified the Product class, but if you think you’re getting shaky while moving functions, run the test case.”
“The next step is to refactor the Product class to invoke the original logic associated with price using a middleman delegate.”
class Product {
constructor(data) {
this._name = data.name;
this._price = new Price(data.price);
/ *... * /
}
get name() {
return this.name;
}
/ *... * /
get price() {
return this._price.toString();
}
get priceCount() {
return this._price.count;
}
get priceUnit() {
return this._price.unit;
}
get priceCnyCount() {
return this._price.cnyCount;
}
get priceSuffix() {
return this._price.suffix; }}Copy the code“When the refactoring is complete, run the test cases.” Geng presses the run key.
“The test case passed. Don’t forget to commit the code.”
“Many people have a preference for basic types, which they generally think are simpler than classes, but don’t let this preference turn into paranoia. Sometimes we need to step out of our traditional caves and into the world of hot objects.”
“This case demonstrates a very common scenario, and I’m sure you’ll be able to identify the basic type of paranoia as a bad taste.”
Xiao Li xiao Wang nodded wildly.
“Let’s look at before and after refactoring.”
“Let’s go on, then.”
Repeated switch
class Price {
constructor(value) {
this._value = value;
}
toString() {
return `The ${this.count} The ${this.suffix}`;
}
get count() {
return parseFloat(this._value.slice(1));
}
get unit() {
switch (this._value.slice(0.1)) {
case A '$':
return 'cny';
case '$':
return 'usd';
case 'k':
return 'hkd';
default:
throw new Error('un support unit'); }}get cnyCount() {
switch (this.unit) {
case 'cny':
return this.count;
case 'usd':
return this.count * 7;
case 'hkd':
return this.count * 0.8;
default:
throw new Error('un support unit'); }}get suffix() {
switch (this.unit) {
case 'cny':
return '元';
case 'usd':
return '$';
case 'hkd':
return 'Hong Kong';
default:
throw new Error('un support unit'); }}}Copy the code“Just now we refined the Price class, now found that the Price class has a problem, do you see?” Lao Geng looked at Xiao Li Xiao Wang.
Xiao Li shook his head, and Xiao Wang said nothing.
“Repetitive switch statements. Be wary whenever you see a switch statement in your code. When you see repetitive switch statements, the bad smell comes out.” “Geng continued.
“The problem with duplicate switches is that every time you want to add a selection branch, you have to find all the switches and update them one by one.”
“And the switch structure is very fragile, and changing the switch statement frequently can cause other problems, which I believe you have encountered.”
Xiao Li seemed to think of something at this point, and added: “The switch statement here is ok, some parts of the switch statement is too long, it is difficult to understand each time, so it is easy to change the problem.”
“Li is right, so let’s reconstruct this Price now. I’m going to be a little lazy here. The test case is the same as the Product test case, and you can write the test case for Price in your actual project. The less granular the test case, the easier it is to locate the problem.”
“Let’s create a factory function and create instance methods of the Product class using the factory function.” Geng started writing code.
class Product {
constructor(data) {
this._name = data.name;
this._price = createPrice(data.price);
/ *... * /
}
/ *... * /
}
function createPrice(value) {
return new Price(value);
}
Copy the code“Run the test case… Ok, yes. Let’s create a subclass CnyPrice, which inherits from Price, and change the factory function to create and return the class CnyPrice when the currency type is ￥.
class CnyPrice extends Price {
constructor(props) {
super(props); }}function createPrice(value) {
switch (value.slice(0.1)) {
case A '$':
return new CnyPrice(value);
default:
return newPrice(value); }}Copy the code“Run the test case… Ok, yes. Then, in the next step, all the conditional logic functions of CNY in Price superclass are rewritten in CnyPrice.”
class CnyPrice extends Price {
constructor(props) {
super(props);
}
get unit() {
return 'cny';
}
get cnyCount() {
return this.count;
}
get suffix() {
return '元'; }}Copy the code“After the rewrite, run the test case… Remove all cny conditional branches from the Price class.
class Price {
constructor(value) {
this._value = value;
}
toString() {
return `The ${this.count} The ${this.suffix}`;
}
get count() {
return parseFloat(this._value.slice(1));
}
get unit() {
switch (this._value.slice(0.1)) {
case '$':
return 'usd';
case 'k':
return 'hkd';
default:
throw new Error('un support unit'); }}get cnyCount() {
switch (this.unit) {
case 'usd':
return this.count * 7;
case 'hkd':
return this.count * 0.8;
default:
throw new Error('un support unit'); }}get suffix() {
switch (this.unit) {
case 'usd':
return '$';
case 'hkd':
return 'Hong Kong';
default:
throw new Error('un support unit'); }}}Copy the code“When the removal is complete, run the test case.”
“Run through, then we do the same, UsdPrice and HkdPrice also create good, finally remove the superclass conditional branch logic related code.” Geng continued to write code.
class Price {
constructor(value) {
this._value = value;
}
toString() {
return `The ${this.count} The ${this.suffix}`;
}
get count() {
return parseFloat(this._value.slice(1));
}
get suffix() {
throw new Error('un support unit'); }}class CnyPrice extends Price {
constructor(props) {
super(props);
}
get unit() {
return 'cny';
}
get cnyCount() {
return this.count;
}
get suffix() {
return '元'; }}class UsdPrice extends Price {
constructor(props) {
super(props);
}
get unit() {
return 'usd';
}
get cnyCount() {
return this.count * 7;
}
get suffix() {
return '$'; }}class HkdPrice extends Price {
constructor(props) {
super(props);
}
get unit() {
return 'hkd';
}
get cnyCount() {
return this.count * 0.8;
}
get suffix() {
return 'Hong Kong'; }}function createPrice(value) {
switch (value.slice(0.1)) {
case A '$':
return new CnyPrice(value);
case '$':
return new UsdPrice(value);
case 'k':
return new HkdPrice(value);
default:
throw new Error('un support unit'); }}Copy the code“When the refactoring is complete, run the test cases.”
“Ok, run through, don’t forget to submit the code.”
“In this way, changing the logic of one currency does not affect the logic of the other currencies, and adding a new currency rule does not affect the logic of the other currencies. Modifying and adding features is easy.”
“Complex conditional logic is one of the hardest things to understand in programming, and it is best to disassemble complex conditional logic by breaking it down into different scenarios. Sometimes the structure of conditional logic is sufficient, but using classes and polymorphisms can make the split of logic more clear.”
“Just like I showed you.”
“Let’s look at before and after refactoring. “
“Let’s go on, then.”
Loop statement (Loop)
function acquireCityAreaCodeData(input, country) {
const lines = input.split('\n');
let firstLine = true;
const result = [];
for (const line of lines) {
if (firstLine) {
firstLine = false;
continue;
}
if (line.trim() === ' ') continue;
const record = line.split(', ');
if (record[1].trim() === country) {
result.push({ city: record[0].trim(), phone: record[2].trim() }); }}return result;
}
Copy the code“Well, let me look at this function, the name seems to be to get the city code information, I want to see the internal implementation of this function. Well, the implementation ignores the first line, then ignores the empty string, then splits the string with commas, and then…”
“It’s a little convoluted, but with a little time you can see the logic.”
“Loops have been a core element of programming since the earliest programming languages. But I feel like the cycle is a bit outdated now.”
“As time goes on, more and more programming languages now offer better language constructs to handle iterative processes, such as arrays in Javascript that have many plumbing methods.”
“Yes, ES is already out to ES12.” Xiao Wang regrets, a little learn not to move.
“Haha, there are some new features that help us with refactoring. Let me show you how to do this. Before the demo, let’s add two more test cases.” Geng started writing code.
describe('test acquireCityData'.() = > {
test('acquireCityData should return India city when input India'.() = > {
const input =
',,+00\nMumbai,India,+91 22\n , , \nTianjing,China,+022\n , , \nKolkata,India,+91 33\nBeijing,China,+010\nHyderabad,India,+91 40';
const result = acquireCityData(input, 'India');
expect(result).toStrictEqual([
{
city: 'Mumbai'.phone: '+ 91 22'
},
{
city: 'Kolkata'.phone: '33 + 91'
},
{
city: 'Hyderabad'.phone: '+ 91 40'}]); }); test('acquireCityData should return China city when input China'.() = > {
const input =
',,+00\nMumbai,India,+91 22\n , , \nTianjing,China,+022\n , , \nKolkata,India,+91 33\nBeijing,China,+010\nHyderabad,India,+91 40';
const result = acquireCityData(input, 'China');
expect(result).toStrictEqual([
{
city: 'Tianjing'.phone: '+ 022'
},
{
city: 'Beijing'.phone: '+ 010'}]); }); });Copy the code“After writing the test case, run it… Ok, passed.” Next, prepare for the refactoring.
“For complex functions like this, we choose to take them apart step by step. First, ignore the first line and use slice instead.”
function acquireCityData(input, country) {
let lines = input.split('\n');
const result = [];
lines = lines.slice(1);
for (const line of lines) {
if (line.trim() === ' ') continue;
const record = line.split(', ');
if (record[1].trim() === country) {
result.push({ city: record[0].trim(), phone: record[2].trim() }); }}return result;
}
Copy the code“When the changes are complete, run the test case… Ok, the next step is to filter an empty line. You can use filter here.”
function acquireCityData(input, country) {
let lines = input.split('\n');
const result = [];
lines = lines.slice(1).filter(line= >line.trim() ! = =' ');
for (const line of lines) {
const record = line.split(', ');
if (record[1].trim() === country) {
result.push({ city: record[0].trim(), phone: record[2].trim() }); }}return result;
}
Copy the code“When the changes are complete, run the test case… Ok, the next step is to split the line with split, you can use map.”
function acquireCityData(input, country) {
let lines = input.split('\n');
const result = [];
lines = lines
.slice(1)
.filter(line= >line.trim() ! = =' ')
.map(line= > line.split(', '));
for (const line of lines) {
if (line[1].trim() === country) {
result.push({ city: line[0].trim(), phone: line[2].trim() }); }}return result;
}
Copy the code“When the changes are complete, run the test case… Ok, the next step is to determine the country. You can use filter.
function acquireCityData(input, country) {
let lines = input.split('\n');
const result = [];
lines = lines
.slice(1)
.filter(line= >line.trim() ! = =' ')
.map(line= > line.split(', '))
.filter(record= > record[1].trim() === country);
for (const line of lines) {
result.push({ city: line[0].trim(), phone: line[2].trim() });
}
return result;
}
Copy the code“When the changes are complete, run the test case… Ok, the last step is data assembly, using Map.”
function acquireCityData(input, country) {
let lines = input.split('\n');
return lines
.slice(1)
.filter(line= >line.trim() ! = =' ')
.map(line= > line.split(', '))
.filter(record= > record[1].trim() === country)
.map(record= > ({ city: record[0].trim(), phone: record[2].trim() }));
}
Copy the code“Refactoring completed, run the test case.”
“Test passed, refactoring done, don’t forget to commit the code.”
“When we look at this function after the refactoring, we can see that the pipe operation helps us see the elements being processed and the actions of processing them more quickly.”
“But.” Wang raised his hand. “In terms of performance, loops are better than pipes, right?”
“That’s a good question, but there are three ways to explain it.”
“First of all, this time is used for two things: first, for performance optimization to make the program run faster, and second, for lack of clear understanding of the program.”
“Let me talk about performance optimization first. If you analyze most programs, it takes half the time on half the code. If you optimize all the code equally, 90 percent of the optimization is wasted, because the code you optimize is rarely executed.”
“On the second side, although refactoring may make the software run slower, it also makes it easier to optimize the performance of the software because the refactoring gives people a better understanding of the program.”
“Third, with the development of modern computer hardware and browser technology, many of the refactoring techniques that used to affect performance, such as small functions, now do not affect performance. Previous perceptions of performance impact need to be updated.”
“There’s a higher concept that needs to be introduced, which is to really do a performance analysis of the system with the right performance measurement tools. Even if you fully understand the system, actually measure its performance, not guesswork. You learn something by guessing, but nine times out of ten you’ll be wrong.”
“So here’s my advice: The secret to ‘writing fast software’ in any situation other than real-time systems where performance is critical is to write tunable software first, and then tune it to get enough speed. In the short term, refactoring may slow the software down, but it makes it easier to tune software performance during the optimization phase, and ultimately pays off.”
“Let’s look at before and after refactoring.”
“Let’s move on to the next one.”
Lazy elements
function reportLines(aCustomer) {
const lines = [];
gatherCustomerData(lines, aCustomer);
return lines;
}
function gatherCustomerData(out, aCustomer) {
out.push(["name", aCustomer.name]);
out.push(["location", aCustomer.location]);
}
Copy the code“Some functions can’t be explicitly said to have problems, but they can be optimized. This function, for example, adds structure to code that may have been designed to support change, facilitate reuse, or even just provide better names, but at this point it doesn’t really need that extra layer of structure. Because the name looks exactly like the implementation code.”
“Sometimes it’s not just the overdesign, it’s the refactoring that gets smaller and smaller, and there’s only one function left.”
“Here I just optimized it with inline functions. Write two test cases first.” Geng started writing code.
describe('test reportLines'.() = > {
test('reportLines should return correct array struct when input aCustomer'.() = > {
const input = {
name: 'jack'.location: 'tokyo'
};
const result = reportLines(input);
expect(result).toStrictEqual([
['name'.'jack'],
['location'.'tokyo']]); }); test('reportLines should return correct array struct when input aCustomer'.() = > {
const input = {
name: 'jackli'.location: 'us'
};
const result = reportLines(input);
expect(result).toStrictEqual([
['name'.'jackli'],
['location'.'us']]); }); });Copy the code“Run the test case… Ok, no problem, let’s start refactoring.” Geng started writing code.
function reportLines(aCustomer) {
const lines = [];
lines.push(["name", aCustomer.name]);
lines.push(["location", aCustomer.location]);
return lines;
}
Copy the code“Ok, easy, refactoring is done, let’s run the test case.”
“The use case test passed. If you want to simplify it, you can revise it.”
function reportLines(aCustomer) {
return[['name', aCustomer.name],
['location', aCustomer.location]
];
}
Copy the code“Running test cases… Passed. Submit the code.”
“As you refactor, you find more and more new structures that you can refactor, as I just demonstrated.”
“Redundant elements like this don’t exist to help much, so let them be generous.”
“Let’s look at before and after the refactoring.”
“Here we go.”
Speculative Generality
class TrackingInformation {
get shippingCompany() {return this._shippingCompany; }set shippingCompany(arg) {this._shippingCompany = arg; }get trackingNumber() {return this._trackingNumber; }set trackingNumber(arg) {this._trackingNumber = arg; }get display() {
return `The ${this.shippingCompany}: The ${this.trackingNumber}`; }}class Shipment {
get trackingInfo() {
return this._trackingInformation.display;
}
get trackingInformation() { return this._trackingInformation; }
set trackingInformation(aTrackingInformation) {
this._trackingInformation = aTrackingInformation; }}Copy the code“Well… Take a look at these two logistics classes, while TrackingInformation records logistics companies and logistics order numbers, while Shipment only uses TrackingInformation to manage logistics information, without any other extra work. Why use an additional TrackingInformation to manage logistics information instead of direct management by Shipment?”
“Because Shipment may have other responsibilities.” Xiao Wang said it was his own code. “So, I used an extra class to track logistics information.”
“Good, single duty principle.”
“How long has this Shipment existed? Let me check the code submission record…” Geng looked at the Git message and said, “Well, it has been in existence for two years, so far it doesn’t seem to have any other responsibilities. Should I wait for it for a few more years?”
“This bad taste is very sensitive.” Geng paused and went on: “There are some designs in the system that talk about generality, common statements that we will use one day, and therefore attempt to deal with unnecessary things with various hooks and special cases, which often make the system more difficult to understand and maintain. “
“Before refactoring, let’s write two test cases.” Geng started writing code.
describe('test Shipment'.() = > {
test('Shipment should return correct trackingInfo when input trackingInfo'.() = > {
const input = {
shippingCompany: 'motion'.trackingNumber: '87349189841231'
};
const result = new Shipment(input.shippingCompany, input.trackingNumber).trackingInfo;
expect(result).toBe('顺丰: 87349189841231');
});
test('Shipment should return correct trackingInfo when input trackingInfo'.() = > {
const input = {
shippingCompany: 'its'.trackingNumber: '1281987291873'
};
const result = new Shipment(input.shippingCompany, input.trackingNumber).trackingInfo;
expect(result).toBe('ZHONGtong: 1281987291873');
});
});
Copy the code“Can’t run the test case yet, why?” Lao Geng asked himself and answered: “Because this use case operation is sure to report an error, the current structure of Shipment does not support such a call, so it must be wrong.”
“I’m going to introduce a new concept here, which is TDD-Test-driven development.”
“Test-driven development is thinking of development in two hats: first put on the hat to implement the function, with the aid of testing, quickly implement its function; Put on the refactoring hat and improve code quality by removing redundant code under the protection of testing. Tests drive the entire development process: first, they drive the design of the code and the implementation of its functionality; Then the driver code is redesigned and refactored.”
“Here, we write out the way we want the program to run, and then we backtrack the design with test cases, and when the test cases pass, the function is developed.”
“Let’s refactor the code.” Geng started writing code.
class Shipment {
constructor(shippingCompany, trackingNumber) {
this._shippingCompany = shippingCompany;
this._trackingNumber = trackingNumber;
}
get shippingCompany() {
return this._shippingCompany;
}
set shippingCompany(arg) {
this._shippingCompany = arg;
}
get trackingNumber() {
return this._trackingNumber;
}
set trackingNumber(arg) {
this._trackingNumber = arg;
}
get trackingInfo() {
return `The ${this.shippingCompany}: The ${this.trackingNumber}`; }}Copy the code“I completely removed the TrackingInformation class and used Shipment to directly manage logistics information. After the refactoring is complete, run the test cases.”
“When the use case runs through, make adjustments where previously applied to Shipment. Of course, it’s safer to replace with the ShipmentNew class first and then remove the original class. I’m going to go back to the code and you two are going to evaluate the impact points and refactor it yourself.” Lao Geng backtracked the code.
Xiao Li xiao Wang nodded wildly.
“In terms of code generality design, it’s worth it if all the devices can be used; If you don’t, it’s not worth it. A useless device will only get in your way, so move it.”
“Let’s look at before and after the refactoring.”
“Let’s get on with it.”
Temporary Field
class Site {
constructor(customer) {
this._customer = customer;
}
get customer() {
return this._customer; }}class Customer {
constructor(data) {
this._name = data.name;
this._billingPlan = data.billingPlan;
this._paymentHistory = data.paymentHistory;
}
get name() {
return this._name;
}
get billingPlan() {
return this._billingPlan;
}
set billingPlan(arg) {
this._billingPlan = arg;
}
get paymentHistory() {
return this._paymentHistory; }}// Client 1
{
const aCustomer = site.customer;
// ... lots of intervening code ...
let customerName;
if (aCustomer === 'unknown') customerName = 'occupant';
else customerName = aCustomer.name;
}
// Client 2
{
const plan = aCustomer === 'unknown' ? registry.billingPlans.basic : aCustomer.billingPlan;
}
// Client 3
{
if(aCustomer ! = ='unknown') aCustomer.billingPlan = newPlan;
}
// Client 4
{
const weeksDelinquent = aCustomer === 'unknown' ? 0 : aCustomer.paymentHistory.weeksDelinquentInLastYear;
}
Copy the code“This section of code is, our offline mall service point, in the old customers move new customers have not moved in, there will be no customers temporarily. “In every place where you look for customer information, you need to determine if there is a customer at that service point, and then you can use that judgment to get valid information.”
“ACustomer === ‘unknown’ is a special case, and in this particular case, a lot of temporary fields, or special value fields, are used. This kind of repetitive judgment not only makes it difficult to repeat the code, but also affects the core logic of the code readability, making it difficult to understand.”
“Here, I’m going to remove all the repetitive judgment logic to keep the core logic code pure. Then, I want to bring these temporary fields together in one place and manage them in a unified way. Let’s write two test cases first.”
describe('test Site'.() = > {
test('Site should return correct data when input Customer'.() = > {
const input = {
name: 'jack'.billingPlan: { num: 100.offer: 50 },
paymentHistory: { weeksDelinquentInLastYear: 28}};const result = new Site(new Customer(input)).customer;
expect({
name: result.name,
billingPlan: result.billingPlan,
paymentHistory: result.paymentHistory
}).toStrictEqual(input);
});
test('Site should return empty data when input NullCustomer'.() = > {
const input = {
name: 'jack'.billingPlan: { num: 100.offer: 50 },
paymentHistory: { weeksDelinquentInLastYear: 28}};const result = new Site(new NullCustomer(input)).customer;
expect({
name: result.name,
billingPlan: result.billingPlan,
paymentHistory: result.paymentHistory
}).toStrictEqual({
name: 'occupant'.billingPlan: { num: 0.offer: 0 },
paymentHistory: { weeksDelinquentInLastYear: 0}}); }); });Copy the code“Well, this is TDD again, and the first use case is running, and running is passable.”
“The next thing I wanted to do was implement NullCustomer, which was really easy to implement.”
class NullCustomer extends Customer {
constructor(data) {
super(data);
this._name = 'occupant';
this._billingPlan = { num: 0.offer: 0 };
this._paymentHistory = {
weeksDelinquentInLastYear: 0}; }}Copy the code“When the implementation is complete, run the test case.”
“After I introduce this special case object, I only need to determine the situation that the old Customer has moved out and the new Customer has not moved in when initializing the Site, and decide which Customer to initialize, instead of having to determine once in every place called and introducing so many temporary fields.”
“If it were written, it would look like this piece of pseudocode.”
// initial.js
const site = customer === 'unknown' ? new Site(new NullCustomer()) : new Site(new Customer(customer));
// Client 1
{
const aCustomer = site.customer;
// ... lots of intervening code ...
const customerName = aCustomer.name;
}
// Client 2
{
const plan = aCustomer.billingPlan;
}
// Client 3{}// Client 4
{
const weeksDelinquent = aCustomer.paymentHistory.weeksDelinquentInLastYear;
}
Copy the code“I’m not going to make any actual changes to your code here, so you can go down and tweak it yourself.”
Xiao Li xiao Wang nodded wildly.
“Let’s look at before and after refactoring.”
“Let’s move on to the next one.”
Long Message Chains
const result = a(b(c(1, d(f()))));
Copy the code“I’ll show you the bad smell in handwritten code, like asking one object for another object, which then asks another object, which then asks another object… This is the message chain. In real code, you might just see a long list of value functions or a long list of temporary variables.”
“With this long list of value functions, the refactoring technique you can use is refining functions, like this.”
const result = goodNameFunc();
function goodNameFunc() {
return a(b(c(1, d(f()))));
}
Copy the code“And then give the refined function a good name.”
“There is another situation, which is the delegate relationship, and you need to hide the delegate relationship. I’m not going to expand it, but if you’re interested, you can read the refactoring book. “
“Let’s look at before and after refactoring.”
Let’s move on to the next.”
The Middle Man
class Product {
constructor(data) {
this._name = data.name;
this._price = createPrice(data.price);
/ *... * /
}
get name() {
return this.name;
}
/ *... * /
get price() {
return this._price.toString();
}
get priceCount() {
return this._price.count;
}
get priceUnit() {
return this._price.unit;
}
get priceCnyCount() {
return this._price.cnyCount;
}
get priceSuffix() {
return this._price.suffix; }}Copy the code“Well, I got the Product + Price back, because after two refactorings, it still has a bad taste.”
“Now I have access to information about the price of the Product, all directly through the Product, which is responsible for providing a lot of the interface to the price. As the Price class gets more and more new features, more forwarding functions get annoying, and it’s getting a little annoying.”
“The Product class has almost completely become a middleman, so I now expect callers to use the Price class directly. Let’s write two test cases first.” Geng started writing code.
describe('test Product price'.() = > {
const products = [
{ name: 'apple'.price: '$6' },
{ name: 'banana'.price: 'RMB 7' },
{ name: 'orange'.price: 'k15' },
{ name: 'cookie'.price: '$0.5'}]; test('Product.price should return correct price when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price.toString());
expect(result).toStrictEqual(['$6'.'7 yuan'.'hk $15'.'$0.5']);
});
test('Product.price should return correct priceCount when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price.count);
expect(result).toStrictEqual([6.7.15.0.5]);
});
test('Product.price should return correct priceUnit when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price.unit);
expect(result).toStrictEqual(['usd'.'cny'.'hkd'.'usd']);
});
test('Product.price should return correct priceCnyCount when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price.cnyCount);
expect(result).toStrictEqual([42.7.12.3.5]);
});
test('Product.price should return correct priceSuffix when input products'.() = > {
const input = [...products];
const result = input.map(item= > new Product(item).price.suffix);
expect(result).toStrictEqual(['$'.'元'.'Hong Kong'.'$']);
});
});
Copy the code“The finished test case can’t be run directly, so let’s adjust the Product class to remove the middleman.”
class Product {
constructor(data) {
this._name = data.name;
this._price = createPrice(data.price);
/ *... * /
}
get name() {
return this.name;
}
/ *... * /
get price() {
return this._price; }}Copy the code“When the adjustment is complete, run the test case directly.”
“The test case passed. Don’t forget to check where Product is used.”
“It’s hard to say what level of hiding is appropriate. But by hiding the delegate and removing the middle man, you can tweak it as the system runs. As the code changes, the scale of “proper hiding” changes accordingly.”
“Let’s look at before and after the refactoring.”
“Let’s move on to the next one.”
Insider Trading
class Person {
constructor(name) {
this._name = name;
}
get name() {
return this._name;
}
get department() {
return this._department;
}
set department(arg) {
this._department = arg; }}class Department {
get code() {
return this._code;
}
set code(arg) {
this._code = arg;
}
get manager() {
return this._manager;
}
set manager(arg) {
this._manager = arg; }}Copy the code“In this case, in order to get Person’s department code, both the code and the department head Manager need to get Person. In this way, the caller needs to know extra details about the Department interface, and if the Department class changes the interface, the change will spread to all the clients that use it through the Person object.”
“We all like to build walls between modules, and we hate to exchange a lot of data between modules because it increases the coupling between modules. In reality, some data exchange is inevitable, but I have to minimize it and make it transparent.”
“Next, we write two test cases in the way we expect the program to run.”
describe('test Person'.() = > {
test('Person should return 88 when input Department code 88'.() = > {
const inputName = 'jack'
const inputDepartment = new Department();
inputDepartment.code = 88;
inputDepartment.manager = 'Tom';
const result = new Person(inputName, inputDepartment).departmentCode;
expect(result).toBe(88);
});
test('Person should return Tom when input Department manager Tom'.() = > {
const inputName = 'jack'
const inputDepartment = new Department();
inputDepartment.code = 88;
inputDepartment.manager = 'Tom';
const result = new Person(inputName, inputDepartment).manager;
expect(result).toBe('Tom');
});
});
Copy the code“In the test case, we can get the Person’s departmentCode and department head manager directly from the Person, so next, we refactor the Person class.”
class Person {
constructor(name, department) {
this._name = name;
this._department = department;
}
get name() {
return this._name;
}
get departmentCode() {
return this._department.code;
}
set departmentCode(arg) {
this._department.code = arg;
}
get manager() {
return this._department._manager;
}
set manager(arg) {
this._department._manager = arg; }}Copy the code“I’m just going to do it one step at a time, but as you practice, take small steps to refactor and roll back the code when you find a problem.” Old Geng earnestly said.
Xiao Li xiao Wang nodded wildly.
“Let’s go back to the code. In the code, I’ve hidden the delegate relationship so that the client relies on the Department class. This way, even if the delegate relationship changes in the future, the change will only affect the object-Person class, not all clients directly.”
“Let’s run the test code.”
“Run through, until all the code replacement is complete, can retain access to the Department, after all the code is changed, then completely removed, commit the code.”
“Let’s look at before and after the refactoring.”
“Let’s move on to the next one.”
Large classes
“There’s also a bad taste called a big class, and I don’t need to give you a new example here. The original Product class actually had this problem.”
“If you try to do too much with a single class, you tend to have too many fields in it. Once that happens, duplicate code ensues. Second, too large a class can also make it difficult to understand. Large classes and long functions have similar problems.”
“We found three bad flavors in the Product class: base-type paranoia, repetitive switch, and middleman. In the process of solving these three bad tastes, we also solved the problem of large classes.”
“Refactoring is continuous in small steps, and you can refine the Product class multiple times for methods other than price, which I won’t demonstrate here.”
Xiao Li xiao Wang nodded wildly.
“Let’s move on to the next one.”
Alternative Classes with Different Interfaces
class Employee {
constructor(name, id, monthlyCost) {
this._id = id;
this._name = name;
this._monthlyCost = monthlyCost;
}
get monthlyCost() {
return this._monthlyCost;
}
get name() {
return this._name;
}
get id() {
return this._id;
}
get annualCost() {
return this.monthlyCost * 12; }}class Department {
constructor(name, staff) {
this._name = name;
this._staff = staff;
}
get staff() {
return this._staff.slice();
}
get name() {
return this._name;
}
get totalMonthlyCost() {
return this.staff.map(e= > e.monthlyCost).reduce((sum, cost) = > sum + cost);
}
get headCount() {
return this.staff.length;
}
get totalAnnualCost() {
return this.totalMonthlyCost * 12; }}Copy the code“There’s a bad smell here, which is similar to duplicate code, called similar classes. Here’s a classic example of Employee.”
“In this case, the Employee class and the Department both have a name field, and they both have the concept of monthlyCost and annualCost, so it’s fair to say that these two classes are doing similar things.”
“We can organize such similar classes by refining superclasses to eliminate repetitive behavior.”
“Before we do that, let’s write two test cases based on what we want to achieve in the end.”
describe('test Employee and Department'.() = > {
test('Employee annualCost should return 600 when input monthlyCost 50'.() = > {
const input = {
name: 'Jack'.id: 1.monthlyCost: 50
};
const result = new Employee(input.name, input.id, input.monthlyCost).annualCost;
expect(result).toBe(600);
});
test('Department annualCost should return 888 when input different staff'.() = > {
const input = {
name: 'Dove'.staff: [{ monthlyCost: 12 }, { monthlyCost: 41 }, { monthlyCost: 24 }, { monthlyCost: 32 }, { monthlyCost: 19}};const result = new Department(input.name, input.staff).annualCost;
expect(result).toBe(1536);
});
});
Copy the code“This test case is also failing to run right now because we haven’t completed the Department transformation yet. Next, we first extract the same fields and behaviors of Employee and Department into a superclass Party.”
class Party {
constructor(name) {
this._name = name;
}
get name() {
return this._name;
}
get monthlyCost() {
return 0;
}
get annualCost() {
return this.monthlyCost * 12; }}Copy the code“These two classes have the same field name and the method of calculating annualCost. Because of the use of” monthlyCost “field, I will also extract” monthlyCost “field and return the default value of” 0 “.
“The next step is to streamline the Employee class, which will be distilled to the superclass for inheritance.”
class Employee extends Party {
constructor(name, id, monthlyCost) {
super(name);
this._id = id;
this._monthlyCost = monthlyCost;
}
get monthlyCost() {
return this._monthlyCost;
}
get id() {
return this._id; }}Copy the code“Next, we’ll remodel the Department class, inherit from the Party class, and then simplify it.”
class Department extends Party {
constructor(name, staff) {
super(name);
this._staff = staff;
}
get staff() {
return this._staff.slice();
}
get monthlyCost() {
return this.staff.map(e= > e.monthlyCost).reduce((sum, cost) = > sum + cost);
}
get headCount() {
return this.staff.length; }}Copy the code“That completes the transformation. Run the test case.”
“The test passed. Be sure to wait until the rest of the use of these classes is complete before you submit the code.”
“If you see two similar classes doing similar things, you can use basic inheritance mechanisms to extract the similarities into a superclass.”
“A lot of times, the proper inheritance is what emerges as the program evolves: I find some common elements, I want to pull them together, and there’s a inheritance. So try to refactor small and fast, and then refactor and find new reconfigurable structures.”
“Let’s look at before and after refactoring.”
“Let’s move on to the next one.”
Pure Data Class
class Category {
constructor(data) {
this._name = data.name;
this._level = data.level;
}
get name() {
return this._name;
}
set name(arg) {
this._name = arg;
}
get level() {
return this._level;
}
set level(arg) {
this._level = arg; }}class Product {
constructor(data) {
this._name = data._name;
this._category = data.category;
}
get category() {
return `The ${this._category.level}.The ${this._category.name}`; }}Copy the code“Category is a pure data class, and with a pure data class like this, using literal objects doesn’t seem to be a problem.”
“But pure data classes often mean that the behavior is in the wrong place. For example, in Product there is a behavior that should belong to the Category, which is to convert to strings. If you move the behavior of processing data from elsewhere to the pure data class, you can make the pure data class exist.”
“Let’s start with two simple test cases.” Geng started writing code.
describe('test Category'.() = > {
test('Product.category should return correct data when input category'.() = > {
const input = {
level: 1.name: 'fruit'
};
const result = new Product({ name: 'apple'.category: new Category(input) }).category;
expect(result).toBe('1. Fruit');
});
test('Product.category should return correct data when input category'.() = > {
const input = {
level: 2.name: 'Hot season fruit'
};
const result = new Product({ name: 'apple'.category: new Category(input) }).category;
expect(result).toBe('2. Hot season fruit ');
});
});
Copy the code“After the test case is written, run it… Ok, yes. And then we’re going to move in behaviors that should be in the Category.”
class Category {
constructor(data) {
this._name = data.name;
this._level = data.level;
}
get name() {
return this._name;
}
set name(arg) {
this._name = arg;
}
get level() {
return this._level;
}
set level(arg) {
this._level = arg;
}
toString() {
return `The ${this._level}.The ${this._name}`; }}class Product {
constructor(data) {
this._name = data._name;
this._category = data.category;
}
get category() {
return this._category.toString(); }}Copy the code“Then we run the test case.”
“The use case worked. Don’t forget to commit the code.” Old Geng hit a commit.
“We need to assign behavior to pure data, or use pure data classes to control the reading and writing of data. Otherwise, pure data classes don’t make much sense and should be removed as redundant.”
“Let’s look at before and after refactoring.”
“Let’s move on to the next one.”
A Refuse Bequest
class Party {
constructor(name, staff) {
this._name = name;
this._staff = staff;
}
get staff() {
return this._staff.slice();
}
get name() {
return this._name;
}
get monthlyCost() {
return 0;
}
get annualCost() {
return this.monthlyCost * 12; }}class Employee extends Party {
constructor(name, id, monthlyCost) {
super(name);
this._id = id;
this._monthlyCost = monthlyCost;
}
get monthlyCost() {
return this._monthlyCost;
}
get id() {
return this._id; }}class Department extends Party {
constructor(name) {
super(name);
}
get monthlyCost() {
return this.staff.map(e= > e.monthlyCost).reduce((sum, cost) = > sum + cost);
}
get headCount() {
return this.staff.length; }}Copy the code“As for the bad taste, I want to rework the Employee and Department example to explain it.”
“As you can see in this example, I moved the staff field from Department to Party, but the Employee class doesn’t really care about the staff field. That’s the bad taste of a rejected legacy.”
“The refactoring is as simple as moving the staff field down to the subclass Department that really needs it, just like I did when I finished refining the superclass.”
“If a field or function in a superclass is relevant to only one or a few subclasses, it is best to move it out of the superclass and into a subclass that really cares about it.”
“Nine times out of ten the bad taste is very mild and requires a high level of familiarity with the business to find it.”
“Let’s look at before and after refactoring.”
“Let’s move on to the next one.”
Comments
“And finally, a word about the bad taste of notes.”
“I don’t think annotations are a bad taste, and they’re a good taste, but the problem with annotations is that many people use them as deodorant.”
“You’ll often see a piece of code with long comments, and then realize that the comments only existed because the code was so bad that the programmer who created it didn’t want to care.”
“When you feel the need to write comments, try refactoring and try to make all the comments redundant.”
“If you don’t know what to do, that’s a good time to use notes. In addition to describing future plans, comments can also be used to mark areas where you are unsure. You can write down your “why did xyz” in the notes. This kind of information can help future modifiers, especially those who are forgetful.”
Xiao Li xiao Wang nodded wildly.
“Well, that’s the end of our special training session. When you two get down here, you need to practice a lot. Develop your sensitivity to bad tastes and then zero tolerance for bad tastes.”
summary
The relationship between bad taste and refactoring is a bit like the relationship between design principles and design patterns. Bad taste/design principles are the tao and refactoring/design patterns are the art.
Those who have dao can have a long time, while those who have no dao will fail. Learning needs to know the dao first, and then it can become great. If learning does not know the dao, it will eventually be a small instrument.
That’s why this article covers 24 bad smells in code, rather than refactoring directly. Because only by recognizing the bad taste in the code, can we try to avoid writing bad taste code, truly achieve perfection, and keep the software healthy and evergreen.
If you spot a bad smell in your code, circle the area with test cases, and then use various refactoring techniques to adjust the structure of the software without changing its observable behavior. Deliver the code as soon as it passes the test, ensuring that your system is ready for release at all times.
The old geng prototype is actually “refactoring: improving the design of existing code” of the authors, wang xiao li refers to the team who are often easier to write code like a patch, and then after a period of time always want to overturn the programming to new people (and possibly the old man), whereas dabao like a hand slew a dragon, but dare not face to face with a dragon, a senior engineer.
I think refactoring also takes courage, the courage to try.
Form a complete set of practice
I’ve compiled all the examples on Github, where each bad flavor has its own directory, and the structure of each directory looks something like this.
xx.before.js: The code before refactoringxx.js: Refactored codexx.test.js: Matching test code
Readers are strongly encouraged to follow this tutorial and complete a refactoring exercise on their own to better identify bad smells and master refactoring techniques.
Here is the corresponding link:
- Mysterious names
- Repeat Code
- Long Function
- Long Parameter List
- Global Data
- Mutable Data
- Divergent changes
- Shotgun Surgery
- Feature Envy
- 1. Data Clumps
- Primitive Obsession
- Repeated switch
- Loop statement (Loop)
- Lazy elements
- Speculative Generality
- Temporary Field
- Long Message Chains
- The Middle Man
- Insider Trading
- Large classes
- Alternative Classes with Different Interfaces
- Pure Data Class
- A Refuse Bequest
- Comments
One last thing
If you have already seen this, I hope you still click “like” before you go ~
Your “like” is the greatest encouragement to the author, and can also let more people see this article!
If you found this article helpful, please help to light up the Star on Github to encourage!