[포키] 사다리 구현 5단계 - 실행결과 출력#190
[포키] 사다리 구현 5단계 - 실행결과 출력#190forkyy-dev wants to merge 14 commits intocodesquad-members-2022:seokho-hamfrom
Conversation
- 동일하게 보여야하는 데이터는 View에서 가지고 있도록 변경
추가 - InputUtil에 예외처리하는 메서드 제거
- Controller에서 서비스에게 result를 매개변수로 전달하도록 변경 - User, Result를 담는 자료구조를 List에서 Map으로 변경
- ladder, result, user 패키지로 분리 및 각각 Creator클래스 생성
Item으로 변경 - Ladder 클래스에 포인트를 이동시키는 메서드, 좌우 이동 가능 여부 판별 메서드 추가 - Line에 이전, 다음 요소 반환하는 메서드 추가 - OutputUtil 클래스 출력 메서드, addPadding 메서드 추가
- playGame 메서드를 실행해서 결과를 저장. - GameService 결과를 담는 Map타입의 results 변수선언
- 게터메서드와 헷갈릴 수 있는 메서드명이라고 생각되어 변경했습니다.
wheejuni
left a comment
There was a problem hiding this comment.
수고 많으셨습니다. 👍
코멘트 참고 부탁드리고 아래 질문도 한번 고민해봐주세요.
왼쪽, 오른쪽 방향을 enum 으로 표현할 순 없을까?
| public class UserCreator { | ||
| private static final int USERNAME_MAXLENGTH = 5; | ||
| private static final int USERNAME_MINLENGTH = 1; | ||
| private Map<String, User> users; |
There was a problem hiding this comment.
피드백을 제대로 이해하지 못했는데 설명을 더 부탁드려도 될까요?
현재 User 클래스가 이름을 인스턴스 변수로 가지고 있습니다.
There was a problem hiding this comment.
@wheejuni 사용자로부터 유저이름을 입력받으면 해당 이름을 가진 User 객체에 바로 접근하려고 사용했습니다! List로 목록을 가지고 있으면 매번 리스트를 순회하며 찾는것보다는 Map이 더 효율적일것 같아서요.
근데 UserCreator내에 인스턴스 변수로 가질 필요는 없었겠다는 생각은 드네요.
|
|
||
| import java.util.*; | ||
|
|
||
| public class GameService { |
There was a problem hiding this comment.
이 클래스가 수행하는 기능으로 봤을때 GameService 보다는 Game 그 자체에 더 가까운 것 같고요...
그렇다면 지금처럼 UserCreator 를 주입받는지 등의 고민을 할 필요 없이 게임 그 자체에 필요한 데이터만 보관하는 형태, 즉 지금 형태의 구현을 유지해도 될 것 같습니다.
There was a problem hiding this comment.
이 부분에 대해서 질문이 생겼습니다..ㅎㅎ
리뷰를 보고 서비스 객체였으면 Creator를 다른 방식으로 사용해야하는건지, GameService와 Game은 어떤 차이점이 있는건지 궁금합니다.
네이밍의 문제인건가요?
| this.name = name; | ||
| } | ||
|
|
||
| public static Item createResult(String name) { |
There was a problem hiding this comment.
Item 인데 createResult 라는 이름을 쓰는게 조금 어색해 보이긴 합니다
There was a problem hiding this comment.
처음에 클래스명을 Result로 작성했다가 Item으로 변경했는데 메서드명을 수정을 안했네요🥲
| import java.util.Map; | ||
|
|
||
| public class ItemCreator { | ||
| public static Map<Integer, Item> createResultMap(List<String> results) { |
There was a problem hiding this comment.
모든 메소드가 static 이라면 생성자도 private 으로 가려야할 것 같군요
| private int canMoveLeft(Line line, int userPoint) { | ||
| if (line.getPrev(userPoint) == 1) return -2; | ||
| return 0; | ||
| } | ||
|
|
||
| private int canMoveRight(Line line, int userPoint) { | ||
| if (line.getNext(userPoint) == 1) return 2; | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
move 메서드에서 인덴트를 1로 유지해야겠다는 생각에 유저가 이동할 값을 바로 리턴하도록 작성했고 그래서 리턴타입을 int로 정했습습니다.
|
아직 제가 서비스라는 객체의 역할을 제대로 이해하지 못하고 서비스라는 이름을 붙여서 사용하고 있었나봅니다..ㅎㅎ 이른 아침부터 피드백 해주셔서 감사합니다! 월요일 화이팅하세요😊💪🏻 |
안녕하세요 브라이언!
로또 미션을 진행하느라 사다리미션 5단계를 이제야 제출합니다😅
<기능 추가>
<리뷰 적용>
<고민 사항>
현재 GameService 클래스의 생성자 내에서 각 User, Item, Ladder Creator가 사용되고 있습니다.
코드를 보다보니 서비스에서 직접 이 객체들에 접근해서 사용하는 부분이 별로 좋지 않은 방식이라는 생각이 들어서 어떻게 수정할지 고민하다가 결국 브라이언의 피드백을 받고 싶어서 그냥 제출했습니다.
어떤 방식으로 사용하는게 좋을지 브라이언의 의견이 듣고 싶습니다. 아니면 학습해볼 키워드도 좋습니다!
주말에 pr을 보내서 죄송합니다:) 남은 주말 잘보내세요!