6주차. SOLID 원칙 & 리팩토링: 수정

수정하면서 겪은 문제...

수정한 내용: Situation 클래스

public class Situation {

	public String situationContext;
	private ArrayList<Emotion> buffTarget = new ArrayList<>();
	private ArrayList<Emotion> debuffTarget = new ArrayList<>();
	private text textBox = text.getInstance();

	public Situation (String situationContext) {
		this.situationContext = situationContext;
	}
	public void applyEffect() {
		this.buffTarget();
		this.debuffTarget();
	}
	
	public void buffTarget() {
		for (Emotion e : buffTarget) {
			for (Emotion target : Main.player1.getEmotionList()) {
				if (target.equals(e)){
					target.buff();
				}
			}
		}
	}
	
	public void debuffTarget() {
		for (Emotion e: debuffTarget) {
			for (Emotion target : Main.player2.getEmotionList()) {
				if (target.equals(e)){
					target.debuff();
				}
			}
		}
	}

	public void setBuffTarget (ArrayList<Emotion> target) {
		this.buffTarget = target;
	}

	public void setDebuffTarget(ArrayList<Emotion> target) {
		this.debuffTarget = target;
	}

	public void showSituation() {
		textBox.updateText(this.situationContext);
	}
}

우선 이전에는 불필요하게 Situation을 상속받아서 Monday, Win 등과 같은 클래스를 만들어서, 각 상황을 생성하고 클래스 내의 필드에 버프할 감정과 디버프할 감정을 정의해두었었는데, Situation의 객체들로 만들어 관리하는 코드로 수정했다.

applyEffect()와 같이 메소드가 하는 일을 더 명확하게 정리하고, 외부에서 이를 호출하여 상황에 맞는 버프 및 디버프 효과를 줄 수 있도록 코드를 작성했다.

뭔가 메소드 하나에서 하는 일을 하나로 깔끔하게 정리하고 싶어서 buffTarget()과 debuffTarget()으로 나누고, applyEffect()에서 이 둘을 호출해서 버프/디버프 주는 식으로 만들게되었는데, 이게 좋은 방향인지는 아직 잘 모르겠다..

수정한 내용: Main 클래스 및 메인 메소드

public class Main {
	public static Player player1 = new Player(0);
	public static Player player2 = new Player(1);

	public static Situation s;
	static Scanner sc = new Scanner(System.in);

	public static void main(String[] args) {
		LocalDateTime now = LocalDateTime.now();

		new choosePlayer1(player1, player2).setVisible(true);
		setSituation();
	}

	public static void setWinner (int winnerIndex) {
		winner win = winner.getInstance();
		text t = text.getInstance();

		if (winnerIndex == 2) {
			win.setwinner(player2.list.get(0), player2.list.get(1), player2.list.get(2));
		}
		else win.setwinner(player1.list.get(0), player1.list.get(1), player1.list.get(2));

		Battleview1.getInstance().dispose();
		Battleview2.getInstance().dispose();
		t.dispose();
		win.setVisible(true);

	}

	public static void setSituation() {
		LocalDateTime now = LocalDateTime.now();
		int situation_random = (((int) Math.random()) * 100)+now.getSecond() % 4;

		switch (situation_random) {
			case 0:
				s = new Situation("오늘은 월요일입니다. \n 버프대상: 까칠이, 버럭이 \n 디버프대상: 기쁨이");
				s.setBuffTarget(new ArrayList<>(Arrays.asList(new 까칠이(), new 버럭이())));
				s.setDebuffTarget(new ArrayList<>(Arrays.asList(new 슬픔이())));
				break;
			case 1:
				s = new Situation("오늘은 시험보는 날입니다. \n 버프 대상: 까칠이, 슬픔이 \n 디버프대상: 기쁨이");
				s.setBuffTarget(new ArrayList<>(Arrays.asList(new 까칠이(), new 슬픔이())));
				s.setDebuffTarget(new ArrayList<>(Arrays.asList(new 기쁨이())));
				break;
			case 2: // 시험 보는 날
				s = new Situation("오늘은 전학간 날입니다. \n 버프 대상: 까칠이 \n 디버프대상: 기쁨이");
				s.setBuffTarget(new ArrayList<>(Arrays.asList(new 까칠이())));
				s.setDebuffTarget(new ArrayList<>(Arrays.asList(new 기쁨이())));
				break;
			case 3: // 전학 간 날
				s = new Situation("오늘은 하키 시합에서 이긴 날입니다. \n 버프 대상: 기쁨이 \n 디버프대상:  없음");
				s.setBuffTarget(new ArrayList<>(Arrays.asList(new 기쁨이())));
				break;
		}

		s.applyEffect();
	}
}

나름 정리를하긴 했는데, 여전히 코드가 길다.

그리고 여기서 setSituation() 메소드를 보면서 느낀건데, 확장성도 낮고, 수정하려면 저 긴 메소드를 전부 읽고 수정해야한다. 왠지 Situation 클래스를 상속받고, 전부 Situation 클래스의 메소드만 쓰고, 하위 클래스로 두는 이유가 없어서 이 부분을 수정한건데, 오히려 SOLID 원칙(개방-폐쇄 원칙)에 위배되어서 다시 아래에서 수정했다.

상황을 후에 추가하기 위해서 저 긴 코드를 전부 읽고 수정해야하기 때문에, 좋지 않은 수정인 것 같아서 다시 수정해야겠다는 생각이 났다.

수정 내용 정리

수정1: Situation 클래스 및 setSituation() 메소드

public abstract class Situation {

	public String situationContext;
	private ArrayList<Emotion> buffTarget = new ArrayList<>();
	private ArrayList<Emotion> debuffTarget = new ArrayList<>();
	private text textBox = text.getInstance();

	public void applyEffect() {
		this.buffTarget();
		this.debuffTarget();
	}
	
	public void buffTarget() {
		for (Emotion e : buffTarget) {
			for (Emotion target : Main.player1.getEmotionList()) {
				if (target.equals(e)){
					target.buff();
				}
			}
		}
	}
	
	public void debuffTarget() {
		for (Emotion e: debuffTarget) {
			for (Emotion target : Main.player2.getEmotionList()) {
				if (target.equals(e)){
					target.debuff();
				}
			}
		}
	}

	public abstract void setSituation();

	public void setBuffTarget (ArrayList<Emotion> target) {
		this.buffTarget = target;
	}

	public void setDebuffTarget(ArrayList<Emotion> target) {
		this.debuffTarget = target;
	}

	public void setSituationContext (String situationContext) {
		this.situationContext = situationContext;
	}

	public void showSituation() {
		textBox.updateText(this.situationContext);
	}
}

Situation 클래스를 원래대로 추상 클래스로 만들어주었다. 그리고 추상메소드인 setSituation() 이라는 메소드를 만들어, Situation 클래스를 상속하는 하위 클래스들에서 구체적인 내용을 구현하게 하였다.

하위 클래스 중 하나를 살펴보면 아래와 같다.

public class Monday extends Situation {
    public void setSituation() {
        ArrayList<Emotion> buffList = new ArrayList<>(Arrays.asList(new 까칠이(), new 버럭이()));
        ArrayList<Emotion> debuffList = new ArrayList<>(Arrays.asList(new 까칠이(), new 버럭이()));
        String situationContext = "오늘은 월요일입니다. \n 버프대상: 까칠이, 버럭이 \n 디버프대상: 기쁨이";

        setBuffTarget(buffList);
        setDebuffTarget(debuffList);
        setSituationContext(situationContext);
    }
}

setSituation() 메소드에서 버프를 줄 대상, 디버프를 줄 대상, 상황의 내용을 정하고, Situation 상위 클래스의 메소드를 사용해 필드값을 설정해준다.

Main 클래스의 setSituation() 메소드를 살펴보면

public static void setSituation() {
		LocalDateTime now = LocalDateTime.now();
		int situation_random = (((int) Math.random()) * 100)+now.getSecond() % 4;

		switch (situation_random) {
			case 0:
				s = new Monday();
				break;
			case 1:
				s = new Exam();
				break;
			case 2: // 시험 보는 날
				s = new TransferDay();
				break;
			case 3: // 전학 간 날
				s = new WinHockey();
				break;
		}

		s.setSituation();
		s.applyEffect();
	}

랜덤한 수에 따라, 위에 미리 선언해둔 Situation s 변수에, 구체적인 하위 상황 클래스의 객체를 저장한다. 그리고 이후에 setSituation()메소드와 applyEffect()를 통해서 상위참조하고 있는 상황 클래스의 상황 설정과 버프/디버프 효과 부여를 한다.

수정2: Main 클래스 및 main() 메소드

이전에는 메인 클래스 및 메소드에서 하는 일이 굉장히 많았다. 아마 프로그램의 전반적인 관리같은 부분은 메인 클래스에 전부 몰아넣었던걸로 기억한다.

플레이어 배열을 만들어 메인에서 관리하던걸 클래스로 분리하여 관리하도록해 SRP..? 관련 부분을 실현해보고자 했다.

또 긴 코드를 관련 책임 클래스로 분리하고, 메소드 단위로 분리하는 작업을 했다.

package main;

import emotion.*;
import java.util.*;
import view.*;
import situation.*;
import 플레이어.Player;

import java.time.LocalDateTime;

public class Main {
	public static Player player1 = new Player(0);
	public static Player player2 = new Player(1);

	public static Situation s;
	static Scanner sc = new Scanner(System.in);

	public static void main(String[] args) {

		new choosePlayer1(player1, player2).setVisible(true);
		setSituation();
	}

	public static void setWinner (int winnerIndex) {
		winner win = winner.getInstance();
		text t = text.getInstance();

		if (winnerIndex == 2) {
			win.setwinner(player2.list.get(0), player2.list.get(1), player2.list.get(2));
		}
		else win.setwinner(player1.list.get(0), player1.list.get(1), player1.list.get(2));

		Battleview1.getInstance().dispose();
		Battleview2.getInstance().dispose();
		t.dispose();
		win.setVisible(true);

	}

	public static void setSituation() {
		//...생략..
    }
}

우선 배열을 대신해서, Player 클래스를 활용해 플레이어1과 플레이어2의 객체를 만들어 static으로 선언하였다.

메인 메소드의 역할을 프로그램의 시작부분과, 상황 설정을 위한 메소드 호출 두 가지이다. 프로그램의 시작 및 초기 설정을하면 끝이다...

setWinner()라는 메소드는 원래 메인 메소드에서 하던 승자가 정해졌을 때의 동작을 수행한다. 플레이어를 클래스로 분리해 관리하고 있기에 세부 내용은 조금 달라졌다. 사실 이 메소드를 어디로 이동하는게 좋을지...? 잘 모르겠어서 일단 이 부분은 메인에 남겼다...

이전에 Main 클래스에서 하던, 클릭 이벤트 발생 시 이벤트 관련 내용 처리나 플레이어1과 2가 사용하는 감정 배열을 설정 및 반환하는 메소드들은 Player 클래스로 분리했다.

수정3: Player 클래스

원래 이전에는 생존해있는 플레이어의 캐릭터(감정)이나 그 수 등 상당 부분을 메인 클래스 내에서 해결했다. 이부분을 SRP를 위해서 하나의 클래스를 만들어 해당 클래스에서 담당하도록 수정했다.

package 플레이어;

import emotion.Emotion;
import main.Main;
import java.util.ArrayList;

public class Player {
    private int playerNum;
    private ArrayList<Emotion> emotionList = new ArrayList<>();
    public ArrayList<Emotion> list = new ArrayList<>();
    private int aliveNum = 3;

    public Player (int n) {
        this.playerNum = n;
    }

    public void checkState() {
        ArrayList<Emotion> emotionsToRemove = new ArrayList<>();
        for (Emotion e : emotionList) {
            if (!e.state) {
                emotionsToRemove.add(e);
                aliveNum--;
            }
        }
        emotionList.removeAll(emotionsToRemove);

        if (aliveNum == 0) {
            Main.setWinner((playerNum+1)%2);
        }
    }

    public Emotion getRandomEmotion () {
        int randNum = (((int) Math.random()*100)) % this.aliveNum;
        return emotionList.get(randNum);
    }

    public void addEmotionList(Emotion e) {
        this.emotionList.add(e);
        this.list.add(e);
    }

    public ArrayList<Emotion> getEmotionList() {
        return this.emotionList;
    }
}

우선 플레이어의 필드로 플레이어의 캐릭터(감정) 배열을과, 몇 번째 플레이어인지나 선택한 캐릭터를 승자 출력 시 전달해주기 위해서 해당 필드를 만들었다. aliveNum이라는 생존하는 캐릭터의 수를 기록하는 필드 역시 이곳에 두었다.

checkState() 메소드에서 죽은 캐릭터는 배열에서 삭제하고, 만약 캐릭터가 전멸했으면, 승자를 출력하는 메소드를 호출해 실행한다.

이전에는 랜덤한 캐릭터를 공격하기 위해서 아마 메인에서 이 부분을 실행했던 것 같은데, 현재는 플레이어 클래스에서 랜덤한 공격 대상을 선정받을 수 있도록 getRandomEmotion() 메소드를 정의했다. 공격자가 해당 상대 플레이어의 해당 메소드를 호출해 랜덤하게 공격 캐릭터를 받는다.

//choosePlayer 
...
sadSelectButton.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent e) {
//				handleSelect(2);
				player1.addEmotionList(new 슬픔이());
				if(player1.getEmotionList().size() == 3) {
					setChoosePlayer2();
				}
			}
		});

캐릭터를 선택할 때마다, 플레이어의 캐릭터 배열에 추가되도록 addEmotionList() 메소드를 만들고, 캐릭터 선택 버튼을 클릭 시, 이 메소드를 호출하면서 그 안에 캐릭터(감정)을 주입하도록 만들었다. 이 부분이..? 주입(DI)라고 할 수 있는지는 모르겠지만...?

수정4: View 관련 클래스 수정

joySelectButton = new JButton("기쁨이");
		joySelectButton.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent e) {
				player1.addEmotionList(new 기쁨이());
				if(player1.getEmotionList().size() == 3) {
					setChoosePlayer2();
				}

			}
		});

우선 앞에서 말했듯이, 클릭 이벤트에 클릭한 캐릭터가 플레이어의 캐릭터 배열에 추가되도록 addEmotionList() 메소드에 캐릭터(감정) 객체를 만들어 주입한다.

public Battleview1() {
		setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		setBounds(100, 100, 900, 500);
		contentPane = new JPanel();
		contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
		setContentPane(contentPane);
		contentPane.setLayout(null);

		int index = 0;
		for (Emotion e: Main.player1.getEmotionList()){
			CharUI tmp = new CharUI(e, Main.player2, index++);
			charUIList.add(tmp);
			tmp.setImg(e.getImage1());

			contentPane.add(tmp.hp_bar);
			contentPane.add(tmp.attackButton);
			contentPane.add(tmp.healButton);
			contentPane.add(tmp.wAttackButton);
			contentPane.add(tmp.Img);
			contentPane.revalidate();
		}
	}

	public void update() {
		for (CharUI ui : charUIList) {
			ui.setHp();
		}

		contentPane.revalidate();
		contentPane.repaint();
	}

크게 바뀐 부분은 없는 걸로 기억한다... 그냥 원래 메인 클래스에 선언해둔 캐릭터 배열이 아닌 플레이어 클래스의 캐릭터 배열을 가져와서 써야하기 때문에 그 부분을 수정하고,CharUI를 ArrayList로 수정해주었다.

this.attackButton.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent e) {
				Emotion target = opponent.getRandomEmotion();
				emotion.attack(target);

				Battleview1.getInstance().update();
				Battleview2.getInstance().update();
			}
		});

CharUI의 공격 버튼 같은 경우, 랜덤한 플레이어를 받아오는 방식으로 바뀌었기에 해당 부분을 클릭 이벤트에 반영해 수정했다.

사실 사소하게 여기저기 손 봤던 것 같다. 수정한 뒤에, 안돌아가서 또 거의 갈아엎은 부분도 있다. 사실 앞에 정의한, 인터페이스 관련 문제는 수정 못 했다..ㅎ

그런데 사실 수정을 하긴했는데 이게 SOLID 원칙을 적용했다고 할 수 있는지는 잘 모르겠다...