Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[사다리 미션] 신지훈 미션 제출합니다. #17

Open
wants to merge 40 commits into
base: 0094-gengar
Choose a base branch
from

Conversation

0094-Gengar
Copy link

안녕하세요. 승용님, 혜빈님!
이번에는 리뷰어가 두 분이나 되네요..ㅎㅎ

nextstep 에서는 사다리의 위치를 좌표로 접근하는 힌트(?)를 준 것 같은데 구현 및 활용이 어려울 것 같아서 다른 방식으로 코드를 짰습니다.
'플레이어의 position' 과 '사다리 한 층의 인덱스'를 비교하며 접근했습니다. 사다리의 경우 '|' 와 '연결다리', '빈칸' 모두 요소로서 인덱스가 할당되기 때문에(..?) position <-> "|" 인덱스의 순서를 유의하여 로직을 구현해봤습니다..(글로 설명하기가 어렵네요..;ㅎㅎ)
매주 미션을 진행하다보니 기능 구현은 감이 잡히기 시작한 것 같습니다. 그러나 기능들을 얼마나 작게(?) 나누어 코드를 작성해야할지 감이 잘 안잡히는 것 같습니다.(1메서드 1기능, view 클래스에서 로직 생성 지양 등등,,) 코드에 대한 리뷰도 해주시면 정말 감사하겠지만, 클래스 분리, 로직 분리 등 조금 더 나은(?) 구조를 만들기 위한 리뷰도 해주시면 감사하겠습니다.

시험기간이 막 끝난 참이라 정신이 없을텐데 리뷰 해주셔서 감사합니다!

@혜빈님 제가 오늘(30일) 다른 일정 때문에 자정 전에 코드를 짤 수 없을 것 같아서 예외 처리는 필요한 부분에 주석으로 내용만 적어뒀고, 테스트 코드는 아직 작성 전입니다. 예외 처리와 테스트 코드 작성은 스터디 시작 전(1.금)까지 완료할 예정입니다!!

@0094-Gengar 0094-Gengar changed the title 0094 gengar [사다리 미션] 신지훈 미션 제출합니다. Oct 30, 2024
src/main/java/view/InputView.java Show resolved Hide resolved
src/main/java/view/InputView.java Outdated Show resolved Hide resolved
src/main/java/view/InputView.java Outdated Show resolved Hide resolved
src/main/java/controller/LadderController.java Outdated Show resolved Hide resolved
src/main/java/controller/LadderController.java Outdated Show resolved Hide resolved
src/main/java/domain/Ladder.java Show resolved Hide resolved
src/main/java/domain/Ladder.java Show resolved Hide resolved
src/main/java/domain/PlayerCollection.java Outdated Show resolved Hide resolved
src/main/java/domain/PlayerCollection.java Outdated Show resolved Hide resolved
src/main/java/domain/RandomGenerator.java Outdated Show resolved Hide resolved
Copy link

@kokodak kokodak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 지훈님! 사다리 미션 고생 많으셨습니다 👍

전반적으로 작성하신 코드에 대해서 리뷰를 남겼어요.

가장 첫 번째로 집중해주셨으면 하는 점은, 프로그래밍 요구사항을 만족하는 거예요. 현재는 기능적 & 기술적인 요구사항이 지켜지지 않은 것 같아요.

요구사항을 만족시키려고 노력하다보면, 지훈님이 언급하셨던 클래스 분리 및 로직 분리에 도움을 받을 수 있을 거예요.

밑에 리뷰에도 공부할 때 참고할만한 레퍼런스 링크를 남겨두었지만, 이곳에도 한번 더 남겨 놓을게요.

지훈님이 자바가 처음인 것으로 알고 있는데, 충분히 잘하고 계십니다.

다만, 아직 더 성장해야 할 지점은 많이 있어요. 이번 사다리 미션이 자바의 마지막 미션인만큼, 더 깊이있는 학습을 해보셨으면 좋겠어요.

이때 공부해놓은 지식은 비단 자바 뿐 아니라, 다른 언어에서도 범용적으로 사용될 수 있는 개념일 거예요.
마지막까지 파이팅입니다! 💪

객체, 역할, 책임, 협력에 대한 글

사다리 코드 레퍼런스

src/main/java/domain/Ladder.java Show resolved Hide resolved
src/main/java/domain/Ladder.java Show resolved Hide resolved
src/main/java/domain/Ladder.java Show resolved Hide resolved
src/main/java/domain/PlayerCollection.java Outdated Show resolved Hide resolved
@@ -0,0 +1,29 @@
package domain;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

domain 패키지의 의미에 대해 고민해보시면 좋을 것 같아요. 무엇이 도메인일까요?

PlayerCollection 클래스는 객체로서 존재하는 클래스라기보다, 단순 유틸성 static 함수를 위해 존재하는 클래스 같아요. 이런 클래스가 domain 패키지에 위치하는 것이 옳은 방향일까요?

도메인이 과연 무엇인지, domain 패키지에는 무엇이 담겨야 하는지를 고민해보고, 그에 대한 지훈님의 결론을 얘기해주세요. 잘 모르겠다면, 이 키워드로 검색해보시면 좋은 자료가 많이 나올 것 같아요!


public static void validatePlayerName(final String playerName) {
for (Player player : players) {
if (!player.getName().equals(playerName) && !playerName.equals("all")) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

player에서 getName을 통해 직접적인 비교를 하기보다, player에게 직접 메시지를 전달하는 게 어떨까요?
https://mangkyu.tistory.com/147

src/main/java/controller/LadderController.java Outdated Show resolved Hide resolved
int widthOfLadder = players.size();
this.ladderCollection = new LadderCollection(heightOfLadder, widthOfLadder);
this.playerMovingLogic = new PlayerMovingLogic(ladderCollection);
this.outputView = new OutputView(ladderCollection, players, inputResults);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드의 네이밍을 보면, 사다리를 만든다 라는 행위를 하는 메서드인 것 같아요.

이 메서드에서 outputView를 만드는 행위는, generateLadders 행위와 어울리지 않는 행위인 것 같습니다.

메서드가 특정 행위에만 집중할 수 있도록 로직을 구성해 주세요.

src/main/java/view/InputView.java Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants