[그리디] 김지우 사다리 미션 제출합니다. #50
Conversation
- 사다리 출력 기능 - Ladder, Line, Link 사다리 구조 담당 클래스
- 사용자로부터 넓이/높이 입력 기능 추가 - Width, Height 클래스로 원시값 포장
- 사용자로부터 넓이/높이 입력 기능 추가 - Width, Height 클래스로 원시값 포장
- 사다리 타기 결과 출력 기능 - 시작 지점 -> 도착 지점 출력
- 참여 사람, 실행 결과 입력 로직 추가 - 결과 입력, 출력 로직 추가
- 메서드 분리 - 입력값 검증 로직, 예외처리 추가
There was a problem hiding this comment.
안녕하세요 지우님 늦은 시간에 리뷰를 올려드리네요 일단 리뷰가 많이 늦어진 점 사과드립니다ㅠㅠ 멤버와 리뷰를 주고 받을 수 있는 몇 안되는 기회인데 너무 죄송해요.. 남은 기간동안 질문이나 토론할 주제 주시면 빠르게 답변 할 수 있도록 노력하겠습니다 언제나 부담없이 질문 주시고 DM 주세요!
저도 많이 부족한 상태라 틀린 점이나 코드를 제대로 이해하지 못한 부분이 있을 수 있어요! 그런 부분은 지우님이 코멘트로 바로잡아 주시거나 다른 의견이 있으시다면 편하게 말해주시면 될 것 같아요. 그리고 개인적인 의견으로 갈릴 수 있는 질문들도 몇 개 작성했으니 답변해 주시면 감사하겠습니다! 서로 의견을 나누면서 새로운 시야가 넓어질 수도 있으니까요😊
이번 미션 정말 수고하셨고 남은 시간동안도 열심히 반영해봅시다! 파이팅이에요👍
| private List<String> readValidatedPlayerNames() { | ||
| List<String> names = inputView.readPlayerNames(); | ||
| for (String name : names) { | ||
| if (name.isEmpty() || name.length() > 5) { | ||
| throw new IllegalArgumentException("참여자 이름은 1자 이상 5자 이하여야 합니다: " + name); | ||
| } | ||
| } | ||
| return names; | ||
| } | ||
|
|
||
| private List<String> readValidatedGameResults(int expectedSize) { | ||
| List<String> results = inputView.readGameResults(); | ||
| if (results.size() != expectedSize) { | ||
| throw new IllegalArgumentException("이름과 결과의 개수가 일치하지 않습니다."); | ||
| } | ||
| return results; | ||
| } |
There was a problem hiding this comment.
입력값 검증을 Controller에서 구현 하셨군요! 그리고 메서드 명을 readValidate+{입력값} 형식으로 결정하셨네요.
저도 평소에 입력값을 검증하는 역학을 어떤 계층에게 넘겨야 할 지 고민을 많이 하는 편이에요! 주로 입력값을 받고 원시값 포장이나 일급 컬렉션을 구현하여 값에 대한 검증 메서드를 추가하여 검증을 구현하고 있어요.
지우님도 Height값이나 Width값 같은 값을 검증하실 경우 원시값 포장을 통해 객체에게 검증 책임을 넘기신 걸로 아시는데 두 방법을 같이 사용하시게 된 이유가 궁금합니다!
| private void handleResultRequest(String request, List<String> names, Ladder ladder, List<String> results) { | ||
| while (!request.equals("all")) {// 입력이 'all'이 아닐 때 | ||
| try { | ||
| int playerIndex = getValidPlayerIndex(request, names); | ||
| outputView.printSingleResult( | ||
| names.get(playerIndex), | ||
| results.get(ladder.getFinalPosition(playerIndex)) | ||
| ); | ||
| } catch (IllegalArgumentException e) { | ||
| System.out.println("잘못된 이름입니다. 다시 입력해주세요."); | ||
| } | ||
| request = inputView.readResultRequest(); | ||
| } | ||
| // 'all' 입력될 때 | ||
| outputView.printAllResults(names, results, ladder); | ||
| } |
There was a problem hiding this comment.
try catch 문을 사용하여 예외를 잘 처리해 주셨네요! 프로그램이 바로 종료되지 않고 다시 입력을 받을 수 있도록 하신 점에서 생각을 많이 하신 게 느껴져요! 😯
There was a problem hiding this comment.
README를 잘 작성해 주셨군요! 사용자가 보고 이해하기 쉬운 좋은 README라고 생각 합니다! 그리고 프로젝트 구조가 변경되어도 크게 고칠 부분이 없다는 점도 좋아요! 👍
| private final InputView inputView; | ||
| private final OutputView outputView; | ||
|
|
||
| public LadderController(InputView inputView, OutputView outputView) { | ||
| this.inputView = inputView; | ||
| this.outputView = outputView; | ||
| } |
There was a problem hiding this comment.
View를 생성자 매개변수로 받아 의존성을 확실하게 표현해주고 있는 점이 좋은 것 같아요
저는 view의 모든 메서드를 static으로 만들어 사용하는데 매개변수로 받는 점이 Controller만 View에 의존할 수 있다는 규칙을 잘 나타내는 것 같습니다.
There was a problem hiding this comment.
Point 클래스는 사다리 클래스를 만들기 위해 아마 가장 깔끔한 구성인 것 같아요! 다른 리뷰에서 언급드렸듯이 enum을 적용하면 미션 요구사항도 맞추고 조금 더 직관적인 코드를 만들 수 있을 것 같습니다!
| String userInput = scanner.nextLine(); | ||
| return Arrays.stream(userInput.split(",")) | ||
| .map(String::trim) | ||
| .filter(name -> !name.isEmpty()) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
콤마 사이에 빈 값이 있을 경우 아예 입력에서 없애도록 하셨네요! 프로그램이 멈춰버리는 상황을 피하기 위한 좋은 방법인 것 같아요.
이런 경우에는 예외를 발생 시킬 수도 있을 것 같은데 지우님은 이런 입력에 대해 예외를 발생시키는 것과 자동으로 처리하고 넘어가는 것 중 어떤 방식이 더 좋다고 생각하시나요?
| private void printNames(List<String> names) { | ||
| for (String name : names) { | ||
| System.out.print(String.format("%-" + SPACE_WIDTH + "s", name)); | ||
| } | ||
| System.out.println(); | ||
| } |
There was a problem hiding this comment.
String.Format을 통해 글자 간 간격을 잘 맞춰주셨네요 👍
| public void printAllResults(List<String> playerNames, List<String> results, Ladder ladder) { | ||
| System.out.println(); | ||
| System.out.println("실행 결과"); | ||
| for (int i = 0; i < playerNames.size(); i++) { | ||
| int finalIndex = ladder.getFinalPosition(i); | ||
| String result = results.get(finalIndex); | ||
| System.out.println(playerNames.get(i) + " : " + result); | ||
| } | ||
| } |
There was a problem hiding this comment.
결과를 출력하는 코드마다 getFinalPosition 메서드를 호출하고 있는 것 같아요! 이런 경우 출력을 할 때마다 사다리를 타는 로직이 실행되는데, 작은 규모에서는 괜찮지만 사다리의 규모가 커지고 입력이 많다면 비효율 적인 코드가 될 수도 있다고 생각해요. 결과를 저장하여 출력하는 방법도 생각해보시면 좋을 것 같습니다!
There was a problem hiding this comment.
현재 테스트 코드에서 random 값을 넣을 때 그냥 seed값을 임의로 지정해서 넣어주고 있어요. 이 부분에 대해 이전에 자동차 경주 미션을 했을 때 random값 테스트에 대해 공부하고 배웠던 인터페이스가 생각나서 적용해보았더니, 제 의도와 다르게 도메인도 너무 늘어나고 코드 전반적인 가독성이 너무 떨어지는 것 같아서 일단 원래 스타일로 복구시켰습니다... 랜덤값을 seed로 지정해서 테스트하고 있는 현 상황에 대한 지환님의 의견은 어떠신가요..?
이 질문에 대한 답을 할때가 왔네요! 솔직히.. 저는 테스트에 랜덤을 집어넣는 것을 포기했습니다 그리고 나아가서 최대한 각각의 기능에 충실하게 테스트를 하면 굳이 랜덤하게 생성하는 걸 테스트 하지 않아도 충분하다고 생각했어요.
현재 상황에서 사다리를 랜덤하게 생성하는 과정에서 랜덤을 통해 확인해야 하는 부분은 사다리가 좌 우 방향이 중복되지 않고 잘 생기는지에 대한 부분이라고 생각해요. 그렇다면 저희가 검증할 수 있는 건 connect함수가 잘 동작하는지 테스트 할 수 있는 것이라고 생각합니다. 비록 100퍼센트 바람직한 테스트는 아니지만 connect가 정확하게 의도한 대로 동작하는 것이 보장된다면 사다리의 조건 또한 잘 된다고 확신할 수 있을 거라 생각해요!
그래서 random을 사용하는 대신 connect 함수를 테스트 할 수 있는 방법을 찾는 것이 더 좋은 방향이라 생각합니다!
이 부분은 저도 다시 한번 생각해 보아야 겠네요🤔
지환님~! 리뷰어로 만나게 되어 반가워요 🙌
지난 로또 미션을 시작할 때 함께 페어 프로그래밍을 하면서, 미션에 접근하는 스타일이나 진행 방식이 생각보다 비슷했던 기억이 납니다. 이번 리뷰를 통해 그때보다 서로 더 발전된 모습으로 성장할 수 있기를 기대합니다.
첫 리뷰이지만 부담 갖지 마시고, 다양한 이야기를 나누면서 서로 배워가면 좋겠습니다.
잘 부탁드립니다! 🙇♀️
구현 과정(feat. 어려웠던 부분) ✏️
이번 미션이 1-5단계 한 번에 제출하도록 요구되었긴 했지만 제 생각보다 난이가 높게 느껴졌던 미션이라 우선 단계별로 차근차근 진행해보려고 했어요.
진행 과정을 간략히 말씀드리자면,,,
사다리 타기의 도메인 요구 분석부터 많은 시간이 들어갔어요. 이번에 지난 미션들을 복기하며 다른 레퍼런스를 덜 참고해보고 싶었지만 생각보다 너무 지체되어 역시나 많은 검색과 자료를 참고하게 되었네요..ㅎㅎ이 결과로 구현한 도메인은 다음과 같습니다 :
Ladder
: 사다리 전체를 표현.
: 여러 개의
Line을 가짐. (일급컬렉션): 시작지점(참여자)에 따른 최종 위치 로직 가짐.
Line
: 사다리의 한 층을 의미.
: 여러
Point를 가짐. (일급컬렉션)Point
:
|하나를 의미.: 현재 위치에서 오른쪽으로 연결되어 있는지의 정보를 가짐.
Width / Height
: 사다리의 너비(=참여자 수)와 높이. (원시값 포장이라는 요구사항때문에 클래스로 생성했습니다)
: Width는 한 라인의 포인트 수, Height는 라인의 개수.
후에 다음 단계 특히 3단계에서 최종 도착지점을 반환하는 로직이 제대로 작동하지 않아서 또 한참 수정을 해보았는데요. 현재 테스트 코드까지 돌려본 결과 문제는 없는 것으로 확인했습니다. 그래도 혹시 더 좋은 방법이나 아이디어 있다면 공유해주세요!
고민 중인 부분 🤔
Enum을 적용하라는 요구사항을 도저히... 어떻게 적용해야 할지 모르겠어서 포기를 했는데요. 혹시 어떻게 접근하셨을지 궁금합니다!
현재 테스트 코드에서 random 값을 넣을 때 그냥 seed값을 임의로 지정해서 넣어주고 있어요. 이 부분에 대해 이전에 자동차 경주 미션을 했을 때 random값 테스트에 대해 공부하고 배웠던 인터페이스가 생각나서 적용해보았더니, 제 의도와 다르게 도메인도 너무 늘어나고 코드 전반적인 가독성이 너무 떨어지는 것 같아서 일단 원래 스타일로 복구시켰습니다... 랜덤값을 seed로 지정해서 테스트하고 있는 현 상황에 대한 지환님의 의견은 어떠신가요..?
최대한 자세히 설명하려고 노력했는데 도움이 되셨을지 모르겠어요 😅
이해하기 어려우시거나 궁금한 부분은 편하게! 알려주세요.
추가로 개선방안이나 좋은 아이디어 있다면 이것 또한 적극적으로 공유해주시면 감사하겠습니다!
화이팅입니다 😄