리팩토링이란
리팩토링(refactoring)은 소프트웨어 공학에서 '결과의 변경 없이 코드의 구조를 재조정함'을 뜻합니다. 가독성을 높이고 유지보수를 편하게 하기 위해 코드를 재개편하는 것을 의미하죠. (수업 자료 발췌문)
클린코드를 만드는 것이 목적이 아니라는 점!!
= 기능을 추가하는 과정이 아닌 가독성, 중복제거, 올바른 상속, 추상화 등의 목적으로 코드의 구조나 코드 모듈을 수정하는 과정 (비교문을 줄이는 것도 포함)
리팩토링 방법
- 리네임 : 식별자 이름 변경 (예: 변수명 개선)
- 추상화 : 공통 기능을 상위 클래스로 분리
- 모듈화 : 코드를 기능별로 분리해 재사용성 증가
상속 문제 해결
현재 코드 상태
교수님께서 상속 관련한 문제가 존재한다고 미리 힌트를 주심
일단 먼저 plantUML로 전체적인 내용 확인하기
문제점
P1. 일단 위의 UML에서도 확인 가능하듯이, 올바르지 않은 상속 문제가 존재함
- Player 클래스를 상속하는 클래스들이 이미 물려받은 필드들을 또 만들어서 가지고 있음
이름~imgFile2 필드 전부 제거가 필요함 - 마찬가지로 무기 클래스를 상속하는 클래스들에서는 필요mp와 데미지 필드 제거가 필요함
P2. 무기로공격 메소드는 내부 변수명(mp)과 무기 종류만 다를 뿐 모두 같은 동작을 하고 있어서 추상화가 필요한 것으로 보임
P3. 중요하진 않지만 사용되지 않는 클래스들까지 import되어 있어서 이 부분도 제거가 필요해보임
P4. 쌍고검과 청강검의 경우 방어확률 제한선이 존재함
그러나 이는 매직 넘버이므로, 사용을 지양하고 변수화할 필요가 있음 그래야 나중에 수정할 때도 용이함
수정 후 & 개선된 점
S1. 중복 필드들을 제거함
S2. 상위 클래스인 Player에서 구현하고 하위 클래스에서 제거
// 수정 전 - 하위 클래스에서 오버라이딩
public void 무기로공격(Player target) {}// 수정 후 - 추상화(공통 기능을 상위 클래스로 분리)
public void 무기로공격(Player target) {
int myMp = this.getMP();
int targetHp = target.getHP();
myMp -= 전용무기.get필요mp();
targetHp -= 전용무기.get데미지();
this.setMP(myMp);
target.setHP(targetHp);
}S3. 사용되지 않는 import 구문들 제거
S4. 쌍고검과 청강검 둘 다 private 필드를 만듦
+ 관우의 static 필드인 String은 사용되지 않는 변수이면서 변수의 의미도 불명확한 이유로 제거함
++ 관우를 제외한 다른 클래스들은 기본 생성자가 없어서 추가함
후기
리팩토링과 클린코드 사이의 경계가 굉장히 모호하게 느껴졌지만 최대한 선을 지키려고 노력했던 것 같다.
처음 해보는 리팩토링이라 신선하고 재미있었던 것 같다. 아마 실무로 가면 굉장히 힘든 작업이겠지만... 좋은 경험이 되었던 것 같다.
코드에 정답이 없다는 건 알지만 정답을 보고 싶다는 마음이 든다.
리팩토링 이후 QnA
Q1. Player 클래스를 상속하는 클래스들은 모두 각자의 전용무기 클래스가 존재하고 두 클래스 사이 관계는 Has-a 관계라고 생각하여 전용무기 필드를 갖고 있어도 괜찮다고 생각했지만, 추상화 이후 쓰이지 않아 난감해짐, 또한 Main에서 객체 생성되는데 가지고 있을 필요가 있나 싶음
A2. 작성자의 의도와 활용도를 모르는 상황에서 정답은 없다.
작성자가 그 has-a 관계를 중요하게 생각하고(의도), 후에 추가될 새로운 메소드를 위해서라면(활용도) 전용무기 필드를 그대로 갖고 있어도 괜찮기 때문이다.
이런 이유가 아니고 후에 사용계획도 딱히 없다면 상황에 따라 제거해도 되는 코드줄이니까
그래서 README를 왜 작성하는지 알게 된 것 같고, 앞으로는 관련된 지침이 없다면 과감하게 제거하고 필요하게 됐을 때에 추가하는 방식이 좋을 것 같다.
Q2. Defend 인터페이스의 방어하기() 메소드도 사실 추상화가 가능했을까 궁금해짐
A2. 가능은 하지만, 반드시 추상화가 필요했는지가 더 중요하다.
모든 문제는 추상화가 가능하기에 맘만 먹으면 깔끔하지 않아도 구현 가능했을 것이다. 그러나 방어하기()의 실제 구현이 클래스마다 동일해야 하고 + 추상화 이후의 코드가 깔끔해야 중복코드 제거와 응집성 제고를 위해 인터페이스에서 default로 추상화가 필요하다고 말할 수 있다.
그렇게 생각해봤을 때, 클래스 각각의 방어하기() 구현 로직이 비슷하고 방어확률 제한만 다르기 때문에 추상화가 필요하고, 가능했다는 결론에 다다른다.
앞으로 이 기준 두 가지를 가지고 판단하면 될 것 같다.
다음은 클로드의 도움을 받은 코드인데, 이렇게 방어하기() 내부를 구성하고 하위 클래스에 get방어확률() (=각각의 private 방어확률 필드 getter) 과 set방어완료(boolean b)의 오버라이딩을 만들어주면 목표 달성이다.
public interface Defend {
int get방어확률(); // 각 무기가 자기 확률을 알려줌
void set방어완료(boolean b); // 각 무기가 자기 상태를 저장
default void 방어하기() {
int 방어확률 = (int)(Math.random() * 100);
set방어완료(방어확률 < get방어확률());
}
}