Skip to content
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
b07a49c
<FEAT> 기능 명세서 파일 추가
davidolleh Oct 30, 2024
f493a05
<FEAT> input, ouput view class 추가
davidolleh Oct 30, 2024
2254785
<FEAT> 연결 관련 enum class들 추가
davidolleh Oct 30, 2024
e0ffcfb
<FEAT> controller 객체 추가
davidolleh Oct 30, 2024
02b6a8d
<FEAT> line 객체 추가
davidolleh Oct 30, 2024
7cf4555
<FEAT> 참여자 객체 추가
davidolleh Oct 30, 2024
dcf7dc4
<FEAT> main 함수 추가
davidolleh Oct 30, 2024
06cc958
<FEAT> 당첨 객체 추가
davidolleh Oct 30, 2024
fc66931
<FEAT> 사다리 객체 추가
davidolleh Oct 30, 2024
2e22fee
<FEAT> 사다리 랜덤 생성 객체 추가
davidolleh Oct 30, 2024
613461c
<FIX> 이름 수정
davidolleh Oct 30, 2024
ed047ab
<FIX> 사다리 랜덤 생성 객체 추상화로 분리
davidolleh Oct 30, 2024
71d47c7
<FIX> nextInt 입력 받는거 nextLine으로 변경
davidolleh Oct 30, 2024
dd7174d
<FEAT> width, height ladder의 field로 추가
davidolleh Oct 30, 2024
5781a34
<FEAT> value object로 변경, 예외처리 추가
davidolleh Oct 30, 2024
9b5c7b5
<FEAT> 사다리 결과 출력 기능 추가
davidolleh Oct 30, 2024
125810f
<FEAT> Prize vo로 변경
davidolleh Oct 30, 2024
92408bb
<FEAT> 실행 결과 기능들 추가
davidolleh Oct 31, 2024
431bd39
<FEAT> 실행 결과 객체 추가
davidolleh Oct 31, 2024
77f88a7
<FEAT> controller 기능 추가
davidolleh Oct 31, 2024
9d693f0
<FEAT> Ladder 자동생성 테스트 추가
davidolleh Oct 31, 2024
91e4ceb
<FEAT> 기능 명세서 추가
davidolleh Oct 31, 2024
b70c3a5
<FIX> Indent 1로 마추려고 수정
davidolleh Oct 31, 2024
c76ef4a
<FIX> tmp 변수로 변경
davidolleh Oct 31, 2024
a09be2f
<FIX> 함수로 분리
davidolleh Oct 31, 2024
9bacca2
<FEAT> 개행문자 추가
davidolleh Oct 31, 2024
d767643
<FEAT> Person, Statistic 관련 test 추가
davidolleh Oct 31, 2024
6aef072
<FIX> warning 제거
davidolleh Oct 31, 2024
5402201
<FIX> 함수로 분리
davidolleh Oct 31, 2024
726700e
<FEAT> prize test 추가
davidolleh Oct 31, 2024
a2f5884
<FEAT> 기능명세서 기능 추가
davidolleh Oct 31, 2024
f567502
<FIX> warning 제거
davidolleh Oct 31, 2024
e278cc8
<REFACTOR> 1. Person 값 객체로 사용하지 않도록 수정, 2. 메서드 이름 동사형으로 수정
davidolleh Nov 12, 2024
bf26804
<REFACTOR> README 구체적으로 추가
davidolleh Nov 14, 2024
915c745
<REFACTOR> 변수 이름 수정
davidolleh Nov 14, 2024
d5eda95
<REFACTOR> 금액의 꽝에 대한 처리를 outputview의 역할로 수정
davidolleh Nov 14, 2024
6bf7754
<REFACTOR> Ladder 생성 로직과 entity 분리
davidolleh Nov 15, 2024
39d71d8
<REFACTOR> Line 이름 수정
davidolleh Nov 16, 2024
0c759b5
<REFACTOR> README 추가
davidolleh Nov 16, 2024
d67a2e9
<REFACTOR> line 특정 rowIndex의 방향 가져오는 함수 구현
davidolleh Nov 16, 2024
936a471
<REFACTOR> Ladder의 필드 RowLine으로 변경
davidolleh Nov 18, 2024
5aa5433
<REFACTOR> main class 이름 수정
davidolleh Nov 18, 2024
7057283
<REFACTOR> LadderGame 실행 구현
davidolleh Nov 18, 2024
d766f09
<REFACTOR> LadderGame 실행 구현
davidolleh Nov 18, 2024
f3dd305
<REFACTOR> Controller, OutputView LadderGame에 맞춰서 수정
davidolleh Nov 18, 2024
1d25eb1
<REFACTOR> 사용하지 않는 파일 삭제
davidolleh Nov 18, 2024
8f5d014
<REFACTOR> person record로 수정
davidolleh Nov 18, 2024
3268173
<REFACTOR> README 추가 작성
davidolleh Nov 18, 2024
19585d2
<REFACTOR> 불필요한 함서 제거
davidolleh Nov 18, 2024
afe1ed8
<REFACTOR> 필드 final로 변경
davidolleh Nov 18, 2024
a866afb
<REFACTOR> class record로 변경
davidolleh Nov 18, 2024
0c974c9
<REFACTOR> class record로 변경
davidolleh Nov 18, 2024
8bda0a9
<REFACTOR> LadderFactory 추상화
davidolleh Nov 18, 2024
4547ffe
<REFACTOR> 오타 수정
davidolleh Nov 20, 2024
7481d5d
<REFACTOR> 참여자 테스트 구현
davidolleh Nov 20, 2024
a08a3c9
<REFACTOR> 파일 삭제
davidolleh Nov 20, 2024
27da6f5
<REFACTOR> README 추가
davidolleh Nov 20, 2024
7e3f2e8
<REFACTOR> 테스트 구현
davidolleh Nov 20, 2024
81e327f
<REFACTOR> 정적 변수러 설정
davidolleh Nov 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md

Choose a reason for hiding this comment

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

리뷰가 처음이여서 부족할 수 있습니다.. 참고 부탁드립니다 :)

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- [x] 연결 여부는 랜덤으로 결정한다.
- [x] 사다리는 라인이 겹치지 않아야 한다.
- [x] 사람의 이름은 1자 이상 5자 이하여야 한다.

Choose a reason for hiding this comment

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

[Request Change]

저는 코드를 볼 때, 리드미와 테스트부터 보는 습관이 있는데요.
제일 코드의 의도가 잘 담겨있을 가능성이 크기 때문입니다.
이 리드미만 읽어선 어떤 프로그램인지 어떻게 사용하는지 파악하기가 힘드네요.
승준님의 어플리케이션을 사랑하는 마음으로 리드미를 조금 더 자세하게 적어볼까요?

Copy link
Author

Choose a reason for hiding this comment

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

README를 구체적으로 작성해 보려고 노력해 봤습니다. 아직 익숙지 않아 필요해보이는 요구사항 있으면 말씀해주시면 감사하겠습니다!

13 changes: 13 additions & 0 deletions src/main/java/LadderGame.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import view.InputView;
import view.OutputView;

public class LadderGame {
public static void main(String[] args) {
LadderGameController ladderGameController = new LadderGameController(

Choose a reason for hiding this comment

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

LadderGame 파일을 하나 더 만들어서 LadderGameController를 생성하는 방식으로 구현하셨네요.
저는 하나의 Applicaiton 파일만 사용해서 파일 안에 여러 메소드들을 호출하는 방식으로 구현했었는데,
이렇게 하나의 파일을 더 만들어 깔끔하게 구현하는 것도 좋은 방식인 것 같아요 :)

new InputView(),
new OutputView()
);

ladderGameController.ladderGame();
}
}
93 changes: 93 additions & 0 deletions src/main/java/LadderGameController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import domain.*;
import view.InputView;
import view.OutputView;

import java.util.List;

public class LadderGameController {

private final InputView inputView;
private final OutputView outputView;

public LadderGameController(InputView inputView, OutputView outputView) {
this.inputView = inputView;
this.outputView = outputView;
}

public void ladderGame() {
List<Person> participants = readParticipants();
List<Prize> prizes = readPrizes();

checkPrizeCountValidation(participants.size(), prizes.size());

int height = readLadderHeight();

Ladder ladder = new Ladder(height, participants.size(), new RandomLadderGeneratorImpl());

outputView.printResult(participants, ladder, prizes);

Statistic statistic = new Statistic(ladder, participants, prizes);

printSpecificParticipantResult(statistic, participants);

outputView.printParticipantsPrizesResult(statistic.getParticipantPrize(), participants);
}

private List<Person> readParticipants() {
outputView.printParticipantInquiry();
List<Person> participants = inputView.readParticipants();
outputView.printEmptyLine();

return participants;
}

private List<Prize> readPrizes() {
outputView.printResultPrizeInquiry();
List<Prize> prizes = inputView.readPrizeResults();
outputView.printEmptyLine();

return prizes;
}

private int readLadderHeight() {
outputView.printLadderHeightInquiry();
int height = inputView.readLadderHeight();
outputView.printEmptyLine();

return height;
}

private String readParticipantName() {
outputView.printParticipantPrizeInquiry();
String name = inputView.readPersonName();
outputView.printEmptyLine();

return name;
}

private void printSpecificParticipantResult(Statistic statistic, List<Person> participants) {
while(true) {
String name = readParticipantName();

if (name.equals("all")) {
break;
}

Person person = new Person(name);
checkParticipantValidation(participants, person);
outputView.printParticipantPrizeResult(statistic.getParticipantPrize(person));
}
}

private void checkPrizeCountValidation(int participantCount, int prizeCount) {
if (participantCount != prizeCount) {
throw new IllegalArgumentException("The number of participants does not match the number of prizes.");
}
}

private void checkParticipantValidation(List<Person> participants, Person person) {
if (!participants.contains(person)) {
throw new IllegalArgumentException("참여자가 아닙니다.");
}
}
}
6 changes: 6 additions & 0 deletions src/main/java/domain/Connection.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package domain;

public enum Connection {
CONNECTED,
UNCONNECTED,
}
Comment on lines +3 to +6

Choose a reason for hiding this comment

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

👍

Comment on lines +3 to +6

Choose a reason for hiding this comment

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

📝 Enum에 함수형 인터페이스 넣어보기 2

함수형 인터페이스를 필드로 선언해서, 각각의 enum마다 다음 동작을 정의할 수 있겠네요!

Suggested change
public enum Connection {
CONNECTED,
UNCONNECTED,
}
public enum Connection {
CONNECTED(Connection::createUnconnected),
UNCONNECTED(Connection::createRandomConnection),
;
private static final ThreadLocalRandom random = ThreadLocalRandom.current();
private static final int RANDOM_BOUND = 10;
private static final int THRESHOLD = 5;
final Supplier<Connection> createNext;
Connection(Supplier<Connection> createNext) {
this.createNext = createNext;
}
static Connection createRandomConnection() {
int randomValue = random.nextInt(RANDOM_BOUND);
if (randomValue < THRESHOLD) {
return Connection.CONNECTED;
}
return Connection.UNCONNECTED;
}
private static Connection createUnconnected() {
return UNCONNECTED;
}
public Connection createNext() {
return createNext.get();
}
}

5 changes: 5 additions & 0 deletions src/main/java/domain/Direction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package domain;

public enum Direction {
LEFT, RIGHT, DOWN
}
72 changes: 72 additions & 0 deletions src/main/java/domain/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package domain;

Choose a reason for hiding this comment

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

domain을 기능별로 나누어서 여러 파일로 구현하셨네요!
저 같은 경우에는 파일 2개로 domain을 구현했는데, 이렇게 더 많이 나누는게 깔끔한 것 같아요.
참고하겠습니다.


import java.util.List;

public class Ladder {
private final List<Line> lines;
private final int ladderWidth;
private final int lineHeight;

Choose a reason for hiding this comment

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

[Comment]

이 필드들은 왜 필요한가요?

private final RandomLadderGenerator randomLadderGenerator;


public Ladder(int lineHeight, int ladderWidth, RandomLadderGenerator randomLadderGenerator) {
this.randomLadderGenerator = randomLadderGenerator;
this.ladderWidth = ladderWidth;
this.lineHeight = lineHeight;

List<Line> lines = this.randomLadderGenerator.generateLadder(lineHeight, ladderWidth);
checkLadderValidation(lines);
this.lines = lines;
}

Choose a reason for hiding this comment

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

[�Comment]

팩토리를 사용하여 복잡한 사다리 생성 로직을 관리하는 주체를 만든 것이 인상적이네요!!

두가지 질문이 있는데요.

  1. randomLadderGenerator가 필드(멤버 변수, 객체의 상태, ...)인 이유가 있나요?
    • 어떤 엔티티(Ladder)와 그 생성로직을 담고있는 클래스(Generator)의 의존성은 어떻게 흐르는게 자연스러워보이시나요?
  2. 팩터리 패턴을 사용하여 생성 로직을 분리했을 때 어떤 장점이 있었나요?

Copy link
Author

Choose a reason for hiding this comment

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

  1. 처음에는 Ladder 생성 또한 Ladder의 역할을 한다는 자연스러운 흐름을 따라가다보니 randomLadderGenerator를 Ladder의 필드로 선언하게 되었습니다. 특정 조건을 가진 생성 로직을 담고 있는 Generator는 entity 내부에서 정의하여 사용하는 것보다 entity 외부에서 사용해 Generator 자체가 Ladder를 생성하는 그림이 자연스러워 보입니다!

https://tecoble.techcourse.co.kr/post/2020-05-26-static-factory-method/
"객체 생성"의 역할 자체가 중요한 경우라면 정적 팩토리 패턴을 사용하여 분리하는 것도 좋은 방법이 될 것이다.
이번 미션에서 팩터리 패턴을 이용하여 사다리를 구현한다면 장점은

  1. 코드의 대한 분리로 인해 가독성이 좋아진다. (생성 과정에서 필요한 변수들을 따로 분리시킬 수도 있다.)
  2. 유연성이 좋아진다.(복잡한 생성 로직에 대한 변경이 있을시 더 유연하게 대처할 수 있을거 같다.)

Copy link
Author

Choose a reason for hiding this comment

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

사다리 미션 중에서 랜덤 사다리 객체 생성 자체가 중요한 역할을 한다고 생각하며, 생성 로직 또한 복잡하다는 생각이 들어 정적 팩토리 패턴을 이용하여 생성 로직을 분리해봤습니다!


public List<Line> getLines() {
return lines;
}

public int getLadderWidth() {
return ladderWidth;
}

public int getLineHeight() {
return lineHeight;
}

private void checkLadderValidation(List<Line> lines) {
for (int i = 0; i < lineHeight; i++) {
checkRowValidation(i, lines);
}
}

private void checkRowValidation(int columnIndex, List<Line> lines) {
List<Direction> rowDirections = lines.stream()
.map(l -> l.getPoints().get(columnIndex))
.toList();

int rowSize = rowDirections.size();
int rightCount = 0;

for (int i = 0; i < rowSize; i++) {
if (rowDirections.get(i) == Direction.RIGHT) {
checkRightValidation(i, rowSize, rowDirections);
rightCount++;
}

if (rowDirections.get(i) == Direction.LEFT) {
checkLeftValidation(rightCount);
rightCount--;
}
}
}

private void checkRightValidation(int columnIndex, int rowSize, List<Direction> rowDirections) {
if (columnIndex == rowSize - 1 || rowDirections.get(columnIndex + 1) != Direction.LEFT) {
throw new RuntimeException("사다리가 잘못 만들어졌습니다.");
}
}

private void checkLeftValidation(int rightCount) {
if (rightCount == 0) {
throw new RuntimeException("사다리가 잘못 만들어졌습니다.");
}
}
}

Choose a reason for hiding this comment

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

[Request Change]

이 부분이 정말 복잡하고 메서드가 기네요.
Ladder가 도메인 중에 제일 큰 애니까 당연한 것일까요? 🤔

제가 생각했을 땐 여기 .getXXX을 사용하는 로직들의 직접적인 연관이 있는 도메인은 따로 있는 것 같은데요.
리드미를 작성하고 디미터의 법칙을 생각하면서 리팩토링 한번 해보면 좋겠어요

Copy link
Author

@davidolleh davidolleh Nov 18, 2024

Choose a reason for hiding this comment

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

처음에 Ladder 안에는 ColumnLine이라는 필드가 있었으나 Ladder에서 가장 표현해줘야 할 것은 Line들의 연결성이라고 생각하게 되었습니다.
Line들의 연결성을 표현해주는 RowLine이라는 class를 만들었으며 하나의 열의 연결성 검증 로직을 Ladder이 아닌 RowLine에 넘겼습니다!

15 changes: 15 additions & 0 deletions src/main/java/domain/Line.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package domain;

import java.util.List;

public class Line {
private final List<Direction> points;

public Line(List<Direction> points) {
this.points = points;
}

public List<Direction> getPoints() {
return points;
}
}
35 changes: 35 additions & 0 deletions src/main/java/domain/Person.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package domain;

import java.util.Objects;

public class Person {
private final String name;
private static final int MAX_LENGTH = 5;

public Person(String name) {
nameValidation(name);
this.name = name;
}

public String getName() {
return name;
}

private void nameValidation(String name) {

Choose a reason for hiding this comment

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

[Comment]

보통 메서드명은 동사를 많이 사용합니다.
validate{정상상태}() 이런식으로 많이 짓는것 같아요

if (name.length() > MAX_LENGTH || name.isEmpty())
throw new IllegalArgumentException("사람 이름은 1글자 이상 5글자 이하여야 합니다.");
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Person person = (Person) o;
return Objects.equals(name, person.name);
}

@Override
public int hashCode() {
return Objects.hashCode(name);
}
}

Choose a reason for hiding this comment

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

[Comment]

클래스 내에 상수, 변수, 생성자, 정적 메서드, 메서드, getter, ... 등을 선언하는 순서를 일정하게 가져가는 것도 코드 가독성에 많은 도움을 준답니다.

선언 순서에 대한 글입니다. 이것이 절대적인 규칙이라고 할 순 없지만, 승준님도 승준님만의 기준으로 순서를 통일시켜주세요

Copy link
Author

Choose a reason for hiding this comment

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

상수, 변수, 생성자, 정적 메소드, 메서드, getter, setter, equals, hashcode, toString
저 스스로 볼려고 단 댓글입니다!

Choose a reason for hiding this comment

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

[Comment]

오? equals hashcode를 재 정의하셨네요.
java 프로그래밍을 하면서 저것들을 재정의하는 습관은 매우 좋은 습관이라는 것에 저는 동의해요.(이펙티브 자바에 나오는 내용에 동의)

근데 이름이 같으면 같은 사람이네요.... (저 그럼 황승준으로 개명할래요.👀)

image
제 IDE에서 Person이 값 객체(VO)같으니까 record로 바꾸라고 아우성을 치고 있네요.
Person은 갑객체 인가요? 그렇게 설계하셨나요?
그렇다면 어떤 위험성이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

스크린샷 2024-11-12 오후 10 30 39

미션에서 이름이 같은 참여자가 동시에 게임을 참여하게 된다면
결과를 측정할 때 이름을 통해서 실행 결과를 확인하는 상황에서 오류가 발생 하겠구나 싶어서
우선 이름이 같으면 같은 사람이라고 판단하게 만들어야지 라는 생각으로 VO로 만들었습니다.

생각해 보게 된다면 중복된 이름의 참여자 입력은 사용자의 몫이라는 생각이 들어 값 객체로 만드는 것이 잘못된 생각이 들었습니다.

갑 객체로 만들게 된다면 Map 같은 자료형에서 Person을 Key로 둔다면 중복된 이름이 존재하는 Person이 있다면 원했던 크기보다 작은 Map이 형성되는 위험성들이 있다는 생각을 하게 되었습니다.

Copy link
Author

Choose a reason for hiding this comment

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

중복된 참여자가 있으면 안된다는 로직을 Participants라는 class로 옮겨 표현하였습니다.

32 changes: 32 additions & 0 deletions src/main/java/domain/Prize.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package domain;

import java.util.Objects;

public class Prize {
private final int cost;

public Prize(int cost) {
this.cost = cost;
}

public String costToString() {
if (cost ==0)
return "꽝";

return String.valueOf(cost);
}

Choose a reason for hiding this comment

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

[Request Change]

Prize 값이 0이면 꽝이군요...

(가짜로) 제가 새로운 요구사항을 드리겠습니다.
승준님의 사다리 게임이 너무 유명해져서, 일본과 중국에서도 서비스를 하게 되었어요...
아이고 ㅜㅠㅠ 여기에 일본어와 중국어도 추가해야겠네요...
울고있는 백엔드 개발자가 보이시나요?
image

Choose a reason for hiding this comment

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

[Request Change]

많은 java 컨벤션에서 if 문에 코드 블럭 생략을 지양합니다.
if문의 범위를 한눈에 보기가 어려워서 그런 것 같아요.

그리고 정렬이 맞지 않네요.
IDE의 자동정렬 기능 사용해보면 좋을 것 같아요

Suggested change
public String costToString() {
if (cost ==0)
return "꽝";
return String.valueOf(cost);
}
public String costToString() {
if (cost == 0) {
return "꽝";
}
return String.valueOf(cost);
}

Copy link
Author

Choose a reason for hiding this comment

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

Prize는 금액을 변수로 갖고 있다는 요구사항을 추가하였고 "꽝"에 대한 표현은 view(프론트)로 역할을 넘겼습니다!



@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Prize prize = (Prize) o;
return cost == prize.cost;
}

@Override
public int hashCode() {
return Objects.hashCode(cost);
}

Choose a reason for hiding this comment

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

[Approve]
👍

}
7 changes: 7 additions & 0 deletions src/main/java/domain/RandomLadderGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package domain;

import java.util.List;

public interface RandomLadderGenerator {
List<Line> generateLadder(int lineHeight, int ladderWidth);
}
Loading