(2022.01.02)
function calculate() {...}
//as-is
var calculateResult = calculate();
useHere(calculateResult);
useThere(calculateResult);
//to-be
useHere(calculate());
useThere(calculate());
이런 경우에 calculate 함수가 여러번 호출되는 것이 성능상 불안해서 calculate의 결과 값을 변수에 저장해 놓는 방식을 사용했었는데 지역변수를 제거하는 것이 좋은 패턴이라고 하니 조금 혼란스럽다. 저 calculate 메서드의 연산이 아주 무거운 연산이라면...?
-> 챕터2에서 말하길 성능이 느려지는 전체 코드 중 극히 일부분이기 때문에 최적화가 필요할 때 그 부분만 찾아서 진행하면 된다고 한다.
(2022.01.16)
'기능 추가'와 '리팩터링' 작업을 명확히 분리해 진행할 것.
리팩터링 시점
리팩터링을 위한 코드리뷰는 Pull Request 방식 보다 Pair Programming 방식이 좋다. : 작성자가 맥락을 설명해줄 수 있고 작성자도 리뷰어의 변경 의도를 제대로 이해할 수 있기 때문.
-> 역으로 말하면 Pull Request 방식으로 리뷰할 때 작성자가 맥락을 잘 남겨놓고 리뷰어도 변경 의도를 잘 설명해야한다고 말할 수 있겠다.
내부 동작을 이해해야 할 필요가 없다면 리팩터링 하지 않는다.
리팩터링을 '클린 코드'나 '바람직한 엔지니어링 습관'처럼 도덕적인 이유로 정당화 하지 말것. 리팩터링의 본질은 경제적인 이유로 하는 것이다.
리팩터링하기 전 오류를 재빨리 잡아내기 위해 자가 테스트 코드를 마련해야 한다.
YAGNI(you aren't going to need it): 필요하기 전에 미리 만들지 말라. 상당수가 쓰이지 않거나 수정을 어렵게 한다.
<명사>
리팩터링: 소프트웨어의 겉보기 동작은 그대로 유지한 채, 코드를 이해하고 수정하기 쉽도록 내부 구조를 변경하는 기법
<동사>
리팩터링(하다): 소프트웨어의 겉보기 동작은 그대로 유지한 채, 여러가지 리팩터링 기법을 적용해서 소프트웨어를 재구성하다.
(2022.01.23)
리팩터링을 언제 시작하고 언제 그만할지를 판단하는 일은 리팩터링의 작동 원리를 아는 것 못지않게 중요하다. 리팩토링을 시작하는 시점은 코드에서 냄새가 날 때, 멈추는 시점은 경험을 통해 판단할 것. 코드에서 나는 악취에는 다음과 같은 종류가 있다. 각 악취의 구체적인 예와 해결 방법은 6~12 챕터에서 제시된다.
(2022.02.05)
(2022.02.13)
함수 추출하기와 함수 인라인하기는 서로 반대되는 리팩터링 기법이다.
각각 다음과 같은 때 사용한다.
! 목적과 구현을 분리하는 방식으로 리팩터링
강조를 위해 사용되는 highlight 함수와 그 구현인 reverse 함수. highlight 함수 대신 reverse를 직접 사용해도 되지만 그러면 '강조'라는 목적이 제대로 드러나지 않을 것이다.void highlight() { reverse(); } void reverse() { ... }
변수 추출하기와 변수 인라인하기는 서로 반대되는 리팩터링 기법이다.
각각 다음과 같은 때 사용한다.
ex) let basePrice = order.basePrice
동일한 기능을 하는 새로운 함수, 변수를 원하는 이름으로 만들고, 기존 함수, 변수에서 호출하도록 한뒤 기존 함수를 호출하는 부분을 차례대로 새로운 함수를 호출하도록 변경.
* 하지만 요즘은 IDE가 좋아져서 기능으로 지원되는 경우가 많다.
변환 함수 : 원본 데이터를 입력 받아서 필요한 정보를 모두 도출한 뒤, 각각을 출력 데이터의 필드에 넣어 반환하는 함수.
여러 함수를 클래스, 변환 함수로 묶는 것은 대체로 동일한 효과를 발휘하므로 어느쪽으로 대부분 어느 방법으로 리팩터링해도 괜찮다. 단, 코드 안에서 원본데이터가 갱신될 때에는 클래스로 묶는 것이 낫다. 변환 함수로 묶으면 가공한 데이터를 새로운 레코드에 저장하므로, 원본 데이터가 수정되면 일관성이 깨질 수 있기 때문이다.
서로 다른 여러 대상을 한꺼번에 다루는 코드에 대해 동작을 여러 단계로 쪼개는 리팩터링.
(2022.02.19)
//as-is
organization = {name: "name", country: "KR"};
//to-be
class Organization {
constructor(data) {
this._name = data.name;
this._country = data.country;
}
get name() {return this._name;}
set name(arg) {this._name = arg;}
get country() {return this._country;}
set country(arg) {this._country = arg;}
//as-is
class Person {
get courses() {return this._courses;}
set courses(aList) {this._courses = aList;}
}
//to-be
class Person {
get courses() {return this._courses.slice();}
addCourse(aCourse) {...}
removeCourse(aCourse) {...}
}
//as-is
orders.filter(o => "high" === o.priority || "rush" === o.priority);
//to-be(priority를 String에서 별도 클래스로 정의)
orders.filter(o => o.priority.higherThan(new Priority("normal")));
//as-is
const basePrice = this._quantity * this._itemPrice;
if(basePrice > 1000)
return basePrice * 0.95;
else
return basePrice * 0.98;
//to-be
get basePrice() {this._quantity * this._itemPrice;}
...
if(this.basePrice > 1000)
return this.basePrice * 0.95;
else
return this.basePrice * 0.98;
//1
class Person {
get officeAreaCode() {return this._officeAreaCode;}
get officeNumber() {return this._officeNumber;}
}
//2
class Person {
get officeAreaCode() {return this._telephoneNumber.areaCode;}
get officeNumber() {return this._telephoneNumber.number;}
}
class TelephoneNumber {
get areaCode() {return this._areaCode;}
get number() {return this._number;}
}
//클래스 추출하기 1->2
//클래스 인라인하기 2->1
//1
manager = aPerson.department.manager;
//2
manager = aPerson.manager;
class Person {
//위임 메서드
get manager() {return this.department.manager;}
}
//위임 숨기기 1->2
//중개자 제거하기 2->1
department
에 대한 의존을 없앨 수 있다.department
의 기능을 많이 사용한다면 위임 메서드가 지나치게 늘어날 수 있다.department
의 기능을 적게 사용할 경우 위임 숨기기를, 많이 사용한다면 중개자 제거하기를 사용해 균형을 맞출 수 있다.(2022.02.23)
어떤 함수가 자신이 속한 모듈 A의 요소들보다 다른 모듈 B의 요소들을 더 많이 참조한다면 모듈 B로 옮겨줘야 마땅하다. 이렇게 하면 캡슐화가 좋아져서, 이 소프트웨어의 나머지 부분은 모듈 B의 세부사항에 덜 의존하게 된다.
//as-is
class ClassA {
get methodA() {...}
get methodB() {return this._a;}
}
//to-be
class ClassB {
get methodA() {...} // ClassA의 methodA를 ClassB로 옮기기
}
class ClassA {
get methodA() {...}
get methodB() {return this._data.a;} //a 필드를 data 클래스로 옮기기
}
//A
result.push(`<p>제목: ${person.photo.title}</p>`);
result.concat(photoData(person.photo));
function phtoData(aPhoto) {
return [
`<p>위치: ${aPhoto.location}</p>`,
`<p>날짜: ${aPhoto.date.toDateString()</p>`,
];
}
//B
result.concat(photoData(person.photo));
function phtoData(aPhoto) {
return [
`<p>제목: ${person.photo.title}</p>`, //함수로 옮기기.
`<p>위치: ${aPhoto.location}</p>`,
`<p>날짜: ${aPhoto.date.toDateString()</p>`,
];
}
인라인코드를 대체하는 함수가 있다면 함수 호출로 대체하는 것이 좋다.
사용중인 프로그래밍 언어의 표준 라이브러리나 플랫폼이 제공하는 API를 잘 파악하고 있으면 좋다. 일반적으로 자신이 짠 코드보다 더 효율적이므로...(항상 그렇진 않다)
//as-is
let appliesToMass = false;
for(const s of states) {
if (s === "MA") appliesToMass = true;
}
//to-be
appliesToMass = states.includes("MA");
반복문 하나로 해결할 수 있는 일을 나누는 것에 거부감이 들 수 있지만, (최적화라던가, 최적화라던가, 최적화라던가...) 반복문 쪼개기는 다음 리팩토링으로 이어지는 디딤돌이 될 수 있다. 최적화는 다 만들고 성능을 테스트 해 본 다음에 고려해도 늦지 않다.
//as-is
let averageAge = 0;
let totalSalary = 0;
for(const p of people) {
averageAge += p.age;
totalSalary += p.salary;
}
averageAge = averageAge / people.length;
//to-be
let totalSalary = 0;
let for(const p of people) {
totalSalary += p.salary;
}
let averageAge = 0;
for (const p of people) {
averageAge += p.age;
}
averageAge = averageAge / people.length;
//better(함수 추출 하기, 반복문을 파이프라인으로 바꾸기)
function totalSalary() {
return people.reduce((total, p) => total + p.salary, 0);
}
function averageAge() {
return Math.average(...people.map(p => p.age));
}
(2022.03.02)
//as-is
let temp = 2 * (height + width);
console.log(temp);
temp = height * width;
console.log(temp);
//to-be
const perimeter = 2 * (height + width);
console.log(perimeter);
const area = height * width;
console.log(area);
...대입이 두 번 이상 이뤄진다면 여러 가지 역할을 수행한다는 신호다. 역할이 둘 이상인 변수가 있다면 쪼개야 한다. 예외는 없다. 역할 하나당 변수 하나다.
//as-is
get discountedTotal() {return this._discountedTotal;}
set discount(aNumber) {
const old = this._discount;
this._discount = aNumber;
this._discountedTotal += old - aNumber;
}
//to-be
get discountedTotal() {return; this._baseTotal - this._discount;}
set discount(aNumber) {this._discount = aNumber;}
...효과가 좋은 방법으로, 값을 쉽게 계산해낼 수 있는 변수들을 모두 제거할 수 있다. 계산 과정을 보여주는 코드 자체가 데이터의 의미를 더 분명히 드러내는 경우도 자주 있으며 변경된 값을 깜박하고 결과 변수에 반영하지 않는 실수를 막아준다.
...불변 데이터 값은 프로그램 외부로 건네줘도 나중에 그 값이 나 몰래 바뀌어서 내부에 영향을 줄까 염려하지 않아도 된다. ...그래서 값 객체는 분산 시스템과 동시성 시스템에서 특히 유용하다.
... 일반적으로 같은 데이터를 물리적으로 복제해 사용할 때 가장 크게 문제되는 상황은 그 데이터를 갱신해야 할 때다. 모든 복제본을 찾아서 빠짐없이 갱신해야 하며, 하나라도 놓치면 데이터 일관성이 깨져버린다. 이런 상황이라면 복제된 데이터들을 모두 참조로 바꿔주는 게 좋다.
//as-is
function potentialEnergy(mass, height) {
return mass * 9.81 * height;
}
//to-be
const STANDARD_GRAVITY = 9.81;
function potentialEnergy(mass, height) {
return mass * STANDARD_GRAVITY * height;
}
(2022.03.10)
//as-is
if (!aDate.isBefore(plan.summerStart) && !aDate.isAfter(plan.summerEnd))
charge = quantity * plan.summerRate;
else
charge = quantity * plan.regularRate + plan.regularServiceCharge;
//to-be
if(summer())
charge = summerChage();
else
charge = regularCharge();
//as-is
if (anEmployee.seniority < 2) return 0;
if (anEmployee.monthsDisabled > 12) return 0;
if (anEmployee.isPartTime) return 0;
//to-be
if (isNotEligibleForDisability()) return 0;
function isNotEligibleForDisability() {
return ((anEmployee.seniority < 2)
|| (anEmployee.monthDisabled > 12)
|| (anEmployee.isPartTime));
}
//as-is
function getPayAmount() {
let result;
if (isDead)
result = deadAmount();
else {
if (isSeparated)
result = separatedAmount();
else {
if (isRetired)
result = retiredAmount();
else
result - normalPayAmount();
}
}
return result;
}
//to-be
function getPayAmount() {
if (isDead) return deadAmount();
if (isSeparated) return separatedAmount();
if (isRetired) return retiredAmount();
return normalPayAmount();
}
//as-is
function plumage(bird) {
switch (bird.type) {
case '유럽 제비':
return "보통이다";
case '아프리카 제비':
return (bird.numberOfCoconuts > 2) ? "지쳤다" : "보통이다";
case '노르웨에 파랑 앵무":
return (bird.voltage > 100) ? "그을렸다" : "예쁘다";
default:
return "알 수 없다";
}
}
const bird = Bird('아프리카 제비');
console.log(plumage(bird));
//to-be
class Bird {
get plumage() {
return "알 수 없다";
}
}
//...
//javascript는 상속을 하지 않아도
//같은 메서드를 구현하고 있다면 같은 타입으로 취급된다.(duck typing)
class AfricanSwallow {
get plumage() {
return (bird.numberOfCoconuts > 2) ? "지쳤다" : "보통이다";
}
}
//...
const bird = AfricanSwallow();
//or
const bird = Bird();
console.log(bird.plumage());
새의 종류마다 다른 구현의 해야하는 메서드가 plumage
밖에 없다면 as-is를 써도 무방할 것이다. 다만 대부분의 메서드에서 새의 종류마다 다른 구현이 필요해진 다면 to-be와 같이 다형성을 이용해 타입을 구분하는 것이 좋다고 생각한다.
//as-is
if (aCustomer === "미확인 고객") customerName = "거주자";
else customerName = aCustomer.name;
//...코드들
if (aCustomer === "미확인 고객") customerMail = "없음";
else customerMail = aCustomer.mail;
//...코드들
//if (aCustomer === "미확인 고객")...
//to-be
class UnknownCustomer {
get name() {return "거주자";}
get mail() {return "없음";}
//...
}
customerName = aCustomer.name;
customerMail = aCustomer.mail;
이 리팩토링도 마찬가지로 동일한 방식의 구현이 여러번 중복된다면 적용하는게 좋을 것 같다.
//as-is
if (this.discountRate)
base = base - (this.discountRate * base);
//to-be
assert(this.discountRate >= 0);
if (this.discountRate)
base = base - (this.discountRate * base);
프로그래머가 일으킬만한 오류에만 어서션을 사용할 것. 데이터를 외부에서 읽어 온다면 그 값을 검사하는 작업은 어서션의 대상이 아니라 예외 처리로 대응해야 하는 프로그램 로직의 일부로 다뤄야 한다. 어서션은 버그 추적을 돕는 최후의 수단이다.
//as-is
//...코드
function checkForMiscreants() {
let found = false;
for (const p of people) {
if (!found) {
if (p === "조커") {
sendAlert();
found = true;
}
}
}
}
//...코드
//to-be
//...코드
function checkForMiscreants() {
for (const p of people) {
if (p === "조커") {
sendAlert();
return;
}
}
}
//...코드
as-is의 코드는 found가 true가 되더라도 반복문을 끝까지 실행하게 되지만 to-be의 코드는 '조커'를 찾는 순간 함수가 종료된다.
(2022.03.18)
부수효과가 있는 함수와 없는 함수를 구분하는 것이 좋다.
//as-is
function tenPercentRaise(aPerson) {
aPerson.salary = aPerson.salary.multiply(1.1);
}
function fivePercentRaise(aPerson) {
aPerson.salary = aPerson.salary.multiply(1.05);
}
//to-be
function raise(aPerson, factor) {
aPerson.salary = aPerson.salary.multiply(1 + factor);
}
매개변수를 사용해 중복을 없애는 것이 좋다.
다만, 매개변수가 너무 늘어나는 것에는 주의가 필요할 것 같다. 그 경우에는 어떻게 해야할까? 6장에 나오는 매개변수 객체 만들기를 한다던가...
//as-is
const deliveryDate = deliveryDate(anOrder, true);
function deliveryDate(anDate, isRush) {
if (isRush) {
//...
}
else {
//...
}
}
//to-be
const deliveryDate = rushDeliveryDate(anOrder);
function rushDeliveryDate(anOrder) {
//...
}
function regularDeliveryDate(anOrder) {
//...
}
플래그 인수는 호출할 수 있는 함수들이 무엇이고 어떻게 호출해야 하는지를 이해하기거 어려워지게 만든다.
//as-is
const low = aRoom.daysTempRange.low;
const high = aRoom.daysTempRange.high;
if (aPlan.withinRange(low, high)) {
//...
}
//to-be
if (aPlan.withinRange(aRoom.daysTempLange)) {
//...
}
레코드를 통째로 넘기면 변화에 대응하기 쉽다. 하지만 함수가 레코드 자체에 의존하기를 원치 않을 때는 이 리팩터링을 수행하지 않는데, 레코드와 함수가 서로 다른 모듈에 속한 상황이면 특히 더 그렇다.
//매개변수를질의 함수로 바꾸기
//as-is
availableVacation(anEmployee, anEmployee.grade); // 중복이다.
function availableVacation(anEmployee, grade) {
//...
}
//to-be
availableVacation(anEmployee);
function availableVacation(anEmployee) {
const grade = anEmployee.grade;
//...
}
//질의 함수를 매개변수로 바꾸기
//as-is
targetTemperature(aPlan);
function targetTemperature() {
currentTemperature = thermostat.currentTemperature; //전역 변수 참조 중
//...
}
//to-be
targetTemperature(aPlan, thermostat.currentTemperature);
function targetTemperature(aPlan, currentTemperature) {
//...
}
매개변수를 제거하면 값을 결정하는 책임 주체가 달라진다. 매개변수가 있다면 결정 주체가 호출자가 되고, 매개변수가 없다면 피호출 함수가 된다. 피호출 함수가 그 역할을 수행하기에 적합하다면 호출하는 쪽을 간소화 하는 것이 좋다.
//함수
function score(candidate, medicalExam, scoringGuide) {
let result = 0;
let healthLevel = 0;
//...
}
//명령
class Scorer {
constructor(candidate, medicalExam, scoringGuide) {
this._candidate = candidate;
this._medicalExam = medicalExam;
this._scoringGuide = scoringGuide;
}
execute() {
this._result = 0;
this._healthLevel = 0;
//...
}
}
함수를 캡슐화해 그 함수만을 위한 객체로 만든 것을 '명령 객체' 혹은 '명령'이라 한다.
명령은 유현하게 함수를 제어하고 표현할 수 있다.
다만, 유연성은 복잡성을 키울 수 있으므로 사용에 주의해야 한다.
로직이 크게 복잡하지 않다면 명령 객체는 장점보다 단점이 크니 평범한 함수로 바꿔주는게 낫다.
예외는 정확히 예상 밖의 동작일 때만 쓰여야 한다. 예외를 던지는 코드를 프로그램 종료 코드로 바꿔도 프로그램이 여전히 정상 동작할지를 따져보자. 정상 동작하지 않을 것 같다면 예외를 사용하지 말라는 신호다. 예외 대신 오류를 검출하여 프로그램을 정상 흐름으로 되돌리게끔 처리해야 한다.
(2022.03.25)
//as-is
class Order {
get daysToShip() {
return this._warehouse.daysToShip;
}
}
class PriorityOrder extends Order {
get daysToShip() {
return this._priorityPlan.daysToShip;
}
}
//to-be
class Order {
get daysToShip() {
return (this._priorityDelegate)
? this._priorityDelegate.daysToShip
: this._warehouse.daysToShip;
}
}
class PriorityOrderDelegate {
get daysToShip() {
return this._priorityPlan.daysToShip;
}
}
컴포지션(위임)이나 상속 어느 하나만 고집하지 말고 적절히 혼용하라
-본문발췌-
상속
//as-is
class List {...}
class Stack extends List {...}
//to-be
class Stack {
constructor() {
this._storage = new List();
}
}
상속을 잘못 적용한 예로는 자바의 스택 클래스가 유명하다. 자바의 스택은 리스트를 상속하고 있는데, 데이터를 저장하고 조작하는 리스트의 기능을 재활용하겠다는 생각이 초래한 결과다. 재활용이란 관점에서는 좋았지만 이 상속에는 문제가 있다. 리스트의 연산 중 스택에 적용되지 않는 게 많음에도 그 모든 연산이 스택 인터페이스에 그대로 노출되는게 아닌가! 이보다는 스택에서 리스트 객체를 필드에 저장해두고 필요한 기능만 위힘했다면 더 멋졌을 것이다.
...슈퍼클래스의 기능들이 서브클래스에는 어울리지 않는다면 그 기능들을 상속을 통해 이용하면 안된다는 신호다.
...제대로 된 상속이라면 서브클래스가 슈퍼클래스의 모든 기능을 사용함은 물론, 서브클래스의 인스턴스를 슈퍼클래스의 인스턴스로도 취급할 수 있어야 한다. 다시 말해, 슈퍼클래스가 사용되는 모든 곳에서 서브크래스의 인스턴스를 대신 사용해도 이상없이 동작해야 한다.
-본문발췌-
책을 읽기 전에는 항상 as-is 코드의 문제는 명확하고 대응되는 리팩터링 기법을 적용하는 것으로 문제를 해결할 수 있을 것이라 생각했다. 하지만 책을 읽다보니 as-is 코드의 문제가 명확하지 않은 경우나 as-is와 to-be 코드 사이에 트레이드오프가 있어 리팩터링 하기 어려운 경우가 존재했다. 반대가 존재하는 리팩터링 기법들도 많았다. 마법의 표현 같지만 결론은 그때그때 상황에 맞춰 적절한 리팩터링 기법을 판단해야 한다는 것이다. 다행인 점은 잘못된 리팩터링 기법을 적용해 코드의 상태가 더 나빠지더라도 이 코드 또한 리팩터링을 통해 개선할 수 있다는 것이다. 포기하지 말고 계속해서 리팩터링 하자!