1주차 미션 / 서버 4조 김광일 #1
Conversation
| import java.util.List; | ||
|
|
||
| public class CustomLadderCreator implements LadderCreator{ | ||
| Row[] rows; |
There was a problem hiding this comment.
외부 클래스에서 rows 필드에 직접적으로 접근 가능하므로, 접근 제어자를 private final으로 설정하는 것이 나아보입니다. final로 설정하여, 필드에 인스턴스 재할당을 막아줄수 있습니다.
| public class CustomLadderCreator implements LadderCreator{ | ||
| Row[] rows; | ||
| public CustomLadderCreator(LadderSize ladderSize) { | ||
| ladderSize.validate(); |
There was a problem hiding this comment.
생성자에서 ladderSize.validate() 메서드를 실행하는 것보다, ladderSize 객체 내부에서 생성자나 팩터리 메서드를 통해 인스턴스가 생성될때, 내부에서 검증하는 것이 나아보입니다. 프리미티브 타입을 객체화 할때 장점은, 예외 처리를 내부적으로 실행할 수 있는 장점을 갖고 있습니다.
|
|
||
| import ladder.model.Row; | ||
|
|
||
| public interface LadderCreator { |
There was a problem hiding this comment.
공통적인 로직을 갖는 메서드를 인터페이스로 분리해놓은 코드는 좋은 코드 인 것 같습니다!
| import java.util.Random; | ||
|
|
||
| public class RandomLadderCreator implements LadderCreator{ | ||
| Row[] rows; |
There was a problem hiding this comment.
외부 클래스에서 rows 필드에 직접적으로 접근 가능하므로, 접근 제어자를 private final으로 설정하는 것이 나아보입니다. final로 설정하여, 필드에 인스턴스 재할당을 막아줄수 있습니다.
| Row[] rows; | ||
|
|
||
| public RandomLadderCreator(LadderSize ladderSize) { | ||
| ladderSize.validate(); |
There was a problem hiding this comment.
생성자에서 ladderSize.validate() 메서드를 실행하는 것보다, ladderSize 객체 내부에서 생성자나 팩터리 메서드를 통해 인스턴스가 생성될때, 내부에서 검증하는 것이 나아보입니다. 프리미티브 타입을 객체화 할때 장점은, 예외 처리를 내부적으로 실행할 수 있는 장점을 갖고 있습니다.
|
|
||
|
|
||
| public void validate() { | ||
| if (numberOfRows < 1 || numberOfPerson < 1) { |
There was a problem hiding this comment.
if 문안에, 조건을 넣는 것보다, private 메서드로 분리하여 넣는 것이 나아보입니다.
| private final int numberOfRows; | ||
| private final int numberOfPerson; | ||
|
|
There was a problem hiding this comment.
numberOfRows, numberOfPerson 필드는 프리미티브 타입이라, 의미적으로 맞는 프로그램을 작성하기 힘들어보입니다.
또한 두 필드 모두 0 초과의 조건을 가지므로, int 대신 NatrualNumber 클래스로 관리하는 것이 나아보입니다.
| public Position increment(){ | ||
| return new Position(position+1); | ||
| } | ||
|
|
||
| public Position decrement(){ | ||
| return new Position(position-1); | ||
| } |
There was a problem hiding this comment.
두 메서드 모두 new Position() 메서드를 호출하여, position이 정당한 값인지 예외처리가 불가능합니다. return fromValue() 를 호출하는 것이 적절하다 생각합니다.
| LadderSize ladderSize = new LadderSize(3, 5); // 예시로 5*4 크기의 사다리 생성 | ||
| CustomLadderCreator ladderCreator = new CustomLadderCreator(ladderSize); | ||
|
|
||
| // When | ||
| Row[] rows = ladderCreator.getRows(); | ||
|
|
||
| // Then | ||
| assertEquals(3, rows.length); // 행의 개수가 예상대로 생성되었는지 확인 | ||
| for (Row row : rows) { | ||
| assertEquals(5, row.getSize()); // 각 행의 열의 개수가 예상대로 생성되었는지 확인 | ||
| } | ||
| } |
There was a problem hiding this comment.
테스트 코드를 작성할떄, given when then을 사용하시는 것이 좋은 것 같습니다! 좋은 코드네요!
| Row[] rows = new Row[3]; | ||
| for(int i = 0; i < 3; i++) { | ||
| rows[i] = new Row(3); | ||
| } |
There was a problem hiding this comment.
이 부분이 중복되어 @beforeeach 어노테이션을 가진 메서드를 이용하여, 단위 테스트 시작 전 공통으로 실행되는 로직을 갖는 것이 좋아보입니다.
안녕하세요 1주차 미션을 수행한 캬모치입니다.
이번 미션을 수행하며 객체지향이 무엇인가 다시 생각해볼 수 있는 시간이였던 것 같습니다. 간단하게 리팩토링 한 부분에 대해 설명 드리겠습니다.
현재 Ladder에는 사다리 생성, 사다리 타기, 게임 실행과 같이 여러 역할들이 존재하였습니다. 해당 부분은 단일 책임원칙에 맞지 않다 판단하여 역할들을 LadderCreator, LadderRunner, LadderGame과 같이 분리하였습니다.
체지향 생활체조 원칙에 맞게 최대한 리팩토링 할려고 노력했지만 아직 부족한 부분이 많은거 같습니다.
해당 사다리 출력을 위하여 LadderRunner에서 사다리를 타며 출력할 수 있게 구현해두었습니다.
입출력에 해당하는 것들을 view에 정리하여 모아 두었습니다. 정확히 어느 부분까지 입력을 받아 출력을 해야할지 명확하지 않아 부족한 부분이 많은거 같습니다.
이번 미션을 수행하며 어려웠던 부분이 몇가지 있었던 것 같습니다.
피드백을 적극 반영하여 다시 리팩토링 해보도록 하겠습니다. 감사합니다!