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

3단계 - 사다리(게임 실행) #1986

Open
wants to merge 18 commits into
base: pawoo0211
Choose a base branch
from

Conversation

pawoo0211
Copy link

@pawoo0211 pawoo0211 commented Dec 3, 2023

안녕하세요~ 3단계 진행하여 PR 생성합니다.

rowLineNumber는 -----, ----- ...의 개수를 의미해서 참고 하시면 좋을 것 같습니다!

이번 미션을 진행하면서 테스트 코드 작성을 많이 못했네요ㅠㅠ 테스트 코드도 빠르게 개발하기 위해서 어느정도의 테스트 코드 베이스를 만들어야 한다고 생각하는데 주로 리뷰어께서는 주로 어떤 방식을 사용하시는지 궁금합니다!

@mskangg
Copy link

mskangg commented Dec 4, 2023

이번 미션을 진행하면서 테스트 코드 작성을 많이 못했네요ㅠㅠ 테스트 코드도 빠르게 개발하기 위해서 어느정도의 테스트 코드 베이스를 만들어야 한다고 생각하는데 주로 리뷰어께서는 주로 어떤 방식을 사용하시는지 궁금합니다!

테스트 코드 베이스 이게 어떤 의미일까요??
제가 질문을 이해 못했습니다 😭

@mskangg
Copy link

mskangg commented Dec 4, 2023

#1986 (comment)

테스트코드를 작성하는것에 팁을 물어보신거라면 답변드릴게요 😄
제가 TDD 에 익숙하지 않았을 땐 로직을 먼저 작성한 뒤 public 메서드 전부를 테스트코드로 작성했어요!
이렇게하면 어느정도 비즈니스검증이 되는것 같더라구요!

하지만, 테스트코드를 나중에 작성하다보면 결국 테스트하기 힘든구조가 필연적으로 발생하더라구요.. 😭
이런경우 리팩토링을 하라는 코드의 신호(?)로 판단하고 구조를 바꿔서 하거나, 이때 TDD 를 적용하면 어느정도 쉽게 되더라구요!

Copy link

@mskangg mskangg left a comment

Choose a reason for hiding this comment

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

3단계도 잘 구현해주셨네요! 👏
몇가지 코멘트 남겨드렸어요! 그리고 구현로직에 비해 테스트가 적은것 같아요 😅
예외케이스도 몇가지 추가하여 보완하면 좋을것 같습니다!
그럼 화이팅 🔥

Comment on lines +3 to +10
[] 사다리 게임에 참여하는 사람에 이름을 부여할 수 있다.
[] 이름은 최대 5글자로 제한된다.
[] 사람의 이름은 띄어쓰기가 존재하지 않는 쉼표로 구분한다.
[] 사다리의 모양은 |-----|으로 출력된다.
[] 사다리를 출력할 때 사람 이름도 같이 출력한다.
[] 참여하는 사람의 숫자에 따라 사다리의 폭과 사다리의 개수가 달라진다.
[] |-----|-----| 같이 연속적인 사다리는 가능하지 않다.
[] 하나의 라인에는 최소 하나 이상이 사다리가 존재하며 사다리의 개수는 랜덤하다.
Copy link

Choose a reason for hiding this comment

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

- [ ] 체크전
- [X] 체크후
문법은 이렇습니다 😄

Comment on lines +21 to +26
private Lines createLines(LadderRequest request) {
Lines lines = new Lines();
int rowLineNumber = request.getParticipants().length - 1;
IntStream.range(0, request.getHighCount())
.forEach(index -> lines.addLine(new Line(rowLineNumber)));
return lines;
Copy link

Choose a reason for hiding this comment

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

request 에서 도메인 객체를 만들어줄수도 있지 않을까요? 🤔
request.toLines()
개인적으로 dto는 로직이 들어가지 않는 transfer 객체지만, dto 가 갖고있는 상태변수만을 가지고 domain을 만들수 있다면 dto에서 생성해주는 편이에요!
이 부분은 정답이 없으니 상권님이 고민해봐주세요!

Comment on lines +12 to +23
rowLineNumber = participantNumber - 1;
rowLinePosition = RowLinePositions.create(rowLineNumber);
}

public Line(int rowLineNumber) {
this.rowLineNumber = rowLineNumber;
rowLinePosition = RowLinePositions.create(rowLineNumber);
}

public Line(int rowLineNumber, RowLinePositions rowLinePositions) {
this.rowLineNumber = rowLineNumber;
this.rowLinePosition = rowLinePositions;
Copy link

Choose a reason for hiding this comment

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

생성자를 여러개 사용중이네요 👍
이런 경우 주생성자 부생성자를 나누어 사용하는게 좋습니다 :)

관련링크 남겨드리겠습니다
https://velog.io/@injoon2019/%EC%A3%BC-%EC%83%9D%EC%84%B1%EC%9E%90-%EB%B6%80-%EC%83%9D%EC%84%B1%EC%9E%90

Comment on lines +30 to +34
public void movableParticipants(Participants participants) {
participants.getParticipantList()
.stream()
.forEachOrdered(participant -> movableParticipant(participant));
}
Copy link

Choose a reason for hiding this comment

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

값을 꺼내와서 비즈니스를 처리하고 있네요!
getter를 사용하지 않고 메세지를 보내는 방식으로 처리하는건 어떤가요? 🤔

public void addLine(Line line) {
lineList.add(line);
}

public void moveParticipants(Participants participants) {
lineList.stream()
.forEachOrdered(line -> line.movableParticipants(participants));
Copy link

Choose a reason for hiding this comment

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

메서드명이 boolean을 리턴할것 같은데 void 형식이군요!
간단하게 line.move(participants) 로 하는건 어떠신가요?

Comment on lines +14 to +16
public RowLinePositions(List<Boolean> positionList, int rowLineNumber) {
this.positionList = positionList;
this.rowLineNumber = rowLineNumber;
Copy link

Choose a reason for hiding this comment

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

생성자에선 필수값체크정도 하는게 좋아보여요!
컬렉션 빈값이나 음수값체크정도해서 방어하는게 좋아보여요 🙂

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.

2 participants