I decided to write a series of short blog posts about code smells, so here is the first one.
Feature Envy and Data Envy are two similar and typical code smells. One class/component is more
interested in the methods or data of another class. Let’s see it with a dummy example, we have a PhoneNumber class and a Person class.
class PhoneNumber {
private unformattedNumber: string;
constructor(unformattedNumber: string) {
this.unformattedNumber = unformattedNumber;
}
public getAreaCode() {
return this.unformattedNumber.slice(0,3);
}
public getPrefix() {
return this.unformattedNumber.slice(3,6);
}
public getNumber() {
return this.unformattedNumber.slice(6,10);
}
}class Person {
private name: string;
private phoneNumber: PhoneNumber;
constructor(name: string, phoneNumber: string) {
this.phoneNumber = new PhoneNumber(phoneNumber);
this.name = name;
}
public printPhoneNumber() {
console.log("(" +
this.phoneNumber.getAreaCode() + ") " +
this.phoneNumber.getPrefix() + "-" +
this.phoneNumber.getNumber());
}
}Person has a method printPhoneNumber() that is calling 3 methods of another class. The main reason for creating a class is encapsulation, if another class is too envy about the methods of another one, then it clearly do not respect encapsulation. The reason we want to refactor is to reduce the depedency between classes, e.g. implement getAreaCode differently.
Thankfully the solution is simple, we move the printPhoneNumber to the method that is mostly responsible for the phone numbers and make the methods that are not used outside of the class private.
class PhoneNumber {
private unformattedNumber: string;
constructor(unformattedNumber: string) {
this.unformattedNumber = unformattedNumber;
}
private getAreaCode() {
return this.unformattedNumber.slice(0,3);
}
private getPrefix() {
return this.unformattedNumber.slice(3,6);
}
private getNumber() {
return this.unformattedNumber.slice(6,10);
}
public printPhoneNumber() {
console.log("(" +
this.getAreaCode() + ") " +
this.getPrefix() + "-" +
this.getNumber());
}
}class Person {
private name: string;
private phoneNumber: PhoneNumber;
constructor(name: string, phoneNumber: string) {
this.phoneNumber = new PhoneNumber(phoneNumber);
this.name = name;
}
public printPhoneNumber() {
this.phoneNumber.printPhoneNumber();
}
}