리팩토링 실습
저번 주에 배운 SOLID원칙을 기반으로해, 리팩토링 실습을 진행하는 과제. 이전에 Battle 과제를 진행하면서 작성한 코드를 우선 고치기 위해서 다시 살펴봤는데...
우선 분명 작성했던 코드인데, 하나도 기억이 안나서 한참 이것저것 뜯어봤다..ㅎㅎ
우선 위에가 Battle 과제하면서 구상단계에서 쓴 클래스 다이어그램이다. 사실 위에 다이어그램만 보면, 무슨 과제인지, 프로젝트인지 하나도 모르겠는데 정리하자면, 인사이드아웃에 나오는 캐릭터들이 등장하고, 플레이어1과 플레이어2가 각자 플레이할 캐릭터를 3명씩 선택하여 배틀을하는 그런 시나리오로 작성된 프로젝트였다.
사실 대충봐도 여기저기 문제도 많아보였고, 내가 작성했던 코드인데 무슨 기능을 하는지 잘 모르겠기도해서, 우선은 천천히 문제점을 찾아서 정리해보기로 했다.
문제1: Situation 클래스의 의존성 관련 문제
package situation;
import emotion.*;
public class TransferSchool extends Situation {
public TransferSchool() {
this.situation_context = "오늘은 전학간 날입니다. \n 버프 대상: 까칠이 \n 디버프대상: 기쁨이";
this.buff_target.add(new 까칠이());
this.debuff_target.add(new 기쁨이());
}
}위는 situation 클래스의 하위 클래스 중 하나이다. 우선 딱 보이는 SOLID 원칙 관련문제는 buff_target이라는 동적배열에 해당 클래스 내에서 객체를 생성해서 넣어주고있다.
SOLID의 D였던, Dependency를 고려하다면, 버프와 디버프의 타겟인 감정 객체들을 외부에서 주입받아서 넣어준다면? 후에 버프/디버프 대상들을 수정할 때 조금 더 편하게 수정할 수 있게된다.
코드만 수정하려고 했는데, 일단 Situation 클래스와 그 상속관계 자체를 수정해야할 것 같아졌다.
관련 클래스 다이어그램이 위와 같은데, 우선 각 상황을 상속받는 구체적인 상황들을 객체로 만들지 않고, 클래스로 만들어뒀다.
그런데, 사실 각 클래스 간의 차이점은 버프/디버프 타겟밖에 없다. 따라서...? 이 부분은 객체로 만들고, 각 객체의 버프/디버프 타겟을 외부에서 주입해주는 방향으로 수정해보려고 한다.
그리고 지금 깨달았는데, 버프/디버프 효과를주려고 applyeffect()라는 메소드를 만들었는데...? 아마 다른 메소드로 대체되고 삭제되지 않은 것 같다. 이 부분 역시 다시 확인하고 수정하려고 한다.
무슨 용도의 메소드일지 사실 한참 고민했는데, 애초에 메소드가 private이라 사용되지 않는 것 같아 삭제하면될 것 같다.
문제2: Situation 클래스의 메소드 정리
이 부분은 딱히 어떤 SOLID 원칙이랑 상관이 있는지 잘 모르겠지만, 일단 메소드를 간단하게 정리하는게 좋을 것 같아서 정리하려고한다.
public void give_buff() {
applyEffect(buff_target, true); // true는 buff
}
public void give_debuff() {
applyEffect(debuff_target, false); // false는 debuff
}
private void applyEffect(Vector<Emotion> targets, boolean isBuff) {
// 버프 디버프 로직이 겹쳐서 boolean값에 따라 버프 디버프 실행하는 걸로 모듈화
if (targets.isEmpty()) return;
for (Emotion target : targets) {
for (int i = 0; i < Main.player_list.size(); i++) {
for (Emotion p : Main.getPlayers(i)) {
if (p.getClass().equals(target.getClass())) {
// ==가 아닌 equals()로 변경 ... 어떻게든 배운거 우겨넣기?ㅋㅋㅋㅋ
if (isBuff) {
p.buff();
} else {
p.debuff();
}
}
}
}
}
}
아마 당시에 무슨 문제가 있어서 버프/디버프 메소드 내에서 버프와 디버프 주는 것을 처리하지 않고, 따로 applyEffect()라는 메소드를 만들어서 처리한 것 같다. 불필요하게 메소드를 나누지 않고 정리해야할 것 같다...
문제3: Main 클래스 및 main 메소드
public class Main {
//플레이어 벡터 Main 클래스의 전역변수로 선언. default라 다른 패키지에서 못쓰니까 get, set 메소드 만듦
public static Vector<Vector<Emotion>> player_list = new Vector<>();
static {
for (int i = 0; i < 1; i++) {
player_list.add(new Vector<>()); // 2개의 빈 벡터 추가
}
}
//플레이어 벡터 각각 만들면 코드 너무 겹쳐서 그냥 2차원 벡터로함.
static Scanner sc = new Scanner(System.in);
public static void main(String[] args) {
//테스트용 임시 감정 고르기
for(int i=0; i<3; i++) {
System.out.println("감정고르기(3개)");
int n = sc.nextInt();
setPlayers(n, 1);
}
//상황 랜덤으로 주어지게 하기
Situation s = new Situation();
int situation_random = (int)Math.random()*100%2; //상황추가되면 여기 나누는거 바꾸기
switch(situation_random) {
case 0:
s = new Monday();
break;
case 1:
s = new Win();
break;
case 2: //시험 보는 날
case 3: //전학 간 날
}
//상황 관련 상태 안내문 (나중에 텍스트창에 띄워도 나쁘지 않을듯)
s.showSituation();
s.give_buff();
s.give_debuff();
}
//getPlayers: 플레이어 1/2를 인수로 받아서, 플레이어 벡터 리턴
public static Vector<Emotion> getPlayers(int n) {
if (n ==0 ) {
return player_list.get(0);
}
else {
return player_list.get(1);
}
}
//setPlayers: 플레이어 배열에 선택한 감정들 추가해주기 (버튼 클릭 -> 해당 번호 인자로 전달 -> switch문)
public static void setPlayers(int n, int player) {
player--;
switch(n) {
case 1:
player_list.get(player).add(new 기쁨이());
return;
case 2:
player_list.get(player).add(new 슬픔이());
return;
case 3:
player_list.get(player).add(new 버럭이());
case 4:
player_list.get(player).add(new 까칠이());
case 5:
player_list.get(player).add(new 소심이());
}
}
}
메인 메소드에서 플레이어와 관련한 대부분의 작업을 하고 있다. 플레이어 배열을 생성해서 관리하고, 관련 상태값을 반환하고 설정한다. 일단 플레이어 클래스를 분리해 하나 만들어서, 거기서 플레이어1과 플레이어2가 선택한 감정들을 배열로 만들어서 관리하도록하고, 각 감정의 생존 여부와 같은 상태도 거기에서 관리하게해야할 것 같다.
그리고 플레이어 배열을 Vector로 해두었는데, 이 부분을 ArrayList로 바꿀 예정이다.
문제4: Emotion 클래스의 LSP 관련 문제
Emotion 클래스의 코드는 우선 아래와 같다.
public abstract class Emotion implements Attackable, Healable, Buffable, Debuffable {
public String name;
protected int max_hp;
protected int curr_hp;
protected int attack_power;
protected int heal_power;
protected String image1;
protected String image2;
protected Weapon weapon;
public boolean state = true;
protected text t = text.getInstance();
public void show_view(){
System.out.println("====================================");
System.out.printf("%8s %5d %5d %8s\n", name, curr_hp, attack_power, weapon.name);
System.out.println("====================================");
}
public void attack(Emotion target) {
target.setCurr_hp(target.getCurr_hp() - this.attack_power);
String s = this.name + "가 " + target.name + "를 공격합니다!";
t.updateText(s);
}
public void weaponAttack(Emotion target){
System.out.println("이거 실행됨");
target.setCurr_hp(target.getCurr_hp() - this.weapon.getPower());
String s = this.name + "가 " + this.weapon.name + "으로 " +target.name+"를 공격합니다.";
t.updateText(s);
}
public void heal() {
int random_num = (int)(Math.random() * 100 % 2); //2의 배수면 힐 성공, 아니면 실패
if(random_num == 0) {
curr_hp = (this.curr_hp + heal_power) < max_hp ? this.curr_hp+=heal_power : max_hp;
String s = this.name +"가 힐을 성공했습니다.\n"+"hp가 " + heal_power + "만큼 추가되어 " + this.curr_hp + " 됐습니다!";
if (t != null) t.updateText(s);
}
else {
String s = this.name+"가 힐을 실패했습니다ㅠㅠ";
t.updateText(s);
return;
}
}
...아래의 getter, setter 메소드는 우선 생략했다.
일단 해당 클래스가 여러 인터페이스를 상속하고있다. 그런데, 사실 공격 가능한 감정들의 종류가 몇몇 존재하고, 해당 감정들을 상위참조해 공격을 실행하고, 방어 가능한 감정들의 종류가 또 몇몇 존재해, 그 감정들의 방어를 상위참조해서 방어를하지 않는 이상 굳이 필요하지 않는 것 같다.
아마 처음 만들기 시작할 때 시나리오 상, 공격만 할 수 있는 감정/힐만 할 수 있는 감정 등등 나뉘어있었던 것 같은데, 현실적으로 어려워서 그냥 전부 공격/방어/힐을 할 수 있게 수정하면서 생긴 문제인 것 같다.
수정 방안은 우선 그냥 인터페이스 상속을 삭제하거나, 시나리오대로 각 감정이 할 수 있는 일을 분리해서 그 부분을 구현하는...식이다..
정리 및 요약
사실 여기 정리한 문제말고도 문제가 엄청 많다. 우선, 여기저기에 정의된 메소드와 View가 정말 연결이 많이되어있어서, 메소드 조금 수정하면 정말 연결된 부분을 싹 갈아엎어야할 정도로 전체적인 코드의 의존성이 매우 높다. 사실 SOLID 원칙을 위배한 부분이 코드 전체에 걸쳐서 퍼져있다.
일단 천천히 수정할 수 있는만큼 수정해보려고 하지만, 사실 너무 의존성이 높아서, 손대면 싹 갈아엎어야하기도하고, 안 돌아갈 수도 있을 것 같다..ㅎ
