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

[2단계 - 사다리 게임 실행] 도기(김동호) 미션 제출합니다. #179

Merged
merged 73 commits into from
Feb 27, 2023

Conversation

kdkdhoho
Copy link

@kdkdhoho kdkdhoho commented Feb 22, 2023

안녕하세요. 서브웨이 ☺️
2단계도 진행해보았습니다!
바쁘신 와중에 리뷰 남겨주심에 항상 감사드립니다!!🙇‍♂️

미션을 진행하며 궁금한 사항

  1. Results클래스의 findResult 함수를 설계할 때의 고민입니다.
    함수 파라미터로서 외부에서 값 자체를 받을지? 혹은, 객체 자체를 받아서 값을 물어보는 식으로 구현할지 고민됩니다.
    고민하게 된 이유는 다음과 같습니다. 만약, 미래에 Ladder가 가지는 상태가 변경된다면, 해당 함수도 변경해야 하는 수고로움이 있기에, 이러한 점에서는 Ladder 객체를 받는 것이 더 좋다고 생각됩니다.
    하지만, 만약 (String name, int position, ...) 같이 값 자체를 받게 된다면, 도메인 간 협력을 한다는 느낌을 받지 못하고, 때에 따라 파라미터 수가 3개를 초과할 가능성도 있다고 생각합니다.

  2. 1번과 이어지는 질문입니다. 다른 클래스는 모르겠지만 적어도 도메인 객체 내의 함수는 모두 파라미터로서 원시값보다 객체를 받도록 통일시키는 것이 코드를 이해하는 데도 좋고, 도메인 객체 간 협력이 잘 이루어진다고 생각합니다.
    파라미터로 원시값을 받는 것과 객체를 받는 것에 어떠한 차이가 있나요?

Copy link

@joseph415 joseph415 left a comment

Choose a reason for hiding this comment

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

안녕하세요 도기. 2단계 구현잘해주셨습니다 💯

질문에 답은 코멘트로 달았는데, 이해안가시면 추가로 코멘트 달아주세요!

@@ -1,27 +1,22 @@
# java-ladder

Choose a reason for hiding this comment

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

전체적인 도메인 요구사항들이 잘 나열되어 있어서 이해하는데 도움이 되는것 같습니다! 👍

@@ -33,4 +39,12 @@ private void validateLength(String name) {
public String getName() {
return name;
}

@Override
public boolean equals(Object o) {

Choose a reason for hiding this comment

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

name이 같은 같은 player로 하기 위해서 equals를 재정의하신것 같은데, hash 값을 사용하는 Collection에서도 동일하게 동작할까요?

Copy link
Author

Choose a reason for hiding this comment

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

처음에 equals & hashCode 모두 오버라이딩 했었습니다. 하지만, 현재까지는 equals만 사용되기에 사용되지 않는 메서드를 삭제했습니다.

서브웨이가 왜 이런 질문을 남겼을까를 크루들과 공유한 결과, 이펙티브 자바 아이템 11에 해당하는 equals를 재정의하려거든 hashCode도 재정의해라는 내용 때문이라고 결론을 내렸어요.
사실 hashCode라는 함수가 어떤 함수인지 잘 모르고 있어서 지웠던 것 같아요. 추가하고, 해당 내용 공부해보겠습니다!

Copy link
Author

Choose a reason for hiding this comment

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

혹은 equals와 hashCode를 굳이 오버라이딩 할 필요없이, 이름이 같으면 같은 객체임을 반환하는 함수 하나를 만드는 것도 좋은 방법 같아요.

Choose a reason for hiding this comment

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

네 맞습니다.ㅎㅎ

도기가 말씀주신것처럼 hashCode를 재정의하지 않을거라면, 메서드를 만드는 것이 나은것 같아요.
도기와 협업하는 누군가가 Player를 사용할 때, 착각하게 될 수 있습니다! 그러면 큰 버�그로 이어질 수도 있어요!

}

private boolean canMoveLeft(List<Step> steps, int position) {
return position != FIRST_LINE && steps.get(position - 1) == Step.EXIST;

Choose a reason for hiding this comment

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

요거는 제 개인적인 코딩 스타일인데, -1 같은 숫자를 써줄때는

int leftPoisition = position - 1 

요렇게, 변수로 표현을 해주는게 눈으로 코드 따라가기가 편하더라고요~! 간단하게 의견드려요~!

printResults(results);
}

private static void printNames(List<Player> players) {
Copy link

@joseph415 joseph415 Feb 23, 2023

Choose a reason for hiding this comment

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

asString 의 역할이 View라고 생각해서 옮겨지게 된걸까요?! 덕분에, 도메인을 받아서 바로 view에 보여질 데이터로 정제해서 노출되니 뭔가 깔끔한 느낌이 있네요!

지금같은 간단한 코드에서는 도메인을 넘겨줘서 view에서 처리해주는것이 코드가 깔끔해지고, 불필요한 작업이 없어서 좋아보일 수 있습니다. 하지만, view로 도메인이 객체가 나가게 되면 이전에 사용하던 데이터 때문에, 상위호환성을 지켜야해서 비지니스 요구사항이 변경되어도 도메인영역을 변경해야할 때, 쉽게 할 수가 없게됩니다.

만약에, 도기가 이런 내용을 충분히 잘 이해했다면, 따로 변경을 요청드리지는 않을게요~ 하지만, 잘 이 말이 와닿지 않으면, 도메인이 View로 노출되지 않도록 해주세요!

Copy link
Author

@kdkdhoho kdkdhoho Feb 24, 2023

Choose a reason for hiding this comment

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

view로 도메인이 객체가 나가게 되면 이전에 사용하던 데이터 때문에, 상위호환성을 지켜야해서 비지니스 요구사항이 변경되어도 도메인영역을 변경해야할 때, 쉽게 할 수가 없게됩니다.

"도메인이 객체가 나가게 되면" 이라는 말이 와닿지가 않습니다..
도메인에 해당하는 객체의 데이터가 나가는 것을 의미하시나요? 아니면 순수 객체가 나가는 것을 의미하시나요?


이전에 사용하던 데이터 때문에, 상위호환성을 지켜야해서 비지니스 요구사항이 변경되어도 도메인영역을 변경해야할 때, 쉽게 할 수가 없게됩니다.

라는 말의 의미는, 지금처럼 도메인의 데이터를 건네주는 방식은, 도메인 영역과 출력 요구사항을 같이 변경해야하기 때문에 쉽게 할 수 없다라는 의미인가요? 아직 경험을 해보지 못해서 쉽게 와닿지 않는 것 같아요🥲

Copy link

@joseph415 joseph415 Feb 24, 2023

Choose a reason for hiding this comment

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

안녕하세요 도기ㅎㅎ 질문달아주셨는데 메일알림을 못봤네요ㅠ

네네 도메인 객체가 외부로 노출된다 도메인 객체의 데이터가 외부로 나가게 된다 다 같은 말입니다!

만약에 View가 실제 클라이언트에게 노출되는 영역이라고 생각해보죠! 저희가 만드는 서비스가 굉장히 인기가 많아서 100만명의 클라이언트가 생겼다고 가정해볼게요!

근데 마침, 복잡한 비지니스 기획이 추가돼서 도메인을 변경해야할 상황이 생기면, 쉽게 변경이 가능할까요? 도메인 필드를 하나 삭제해야한다면, 100만영의 클라이언트 모두가 영향을 받을 수 있습니다. 따라서 쉽게 변경이 불가능해집니다. 하위호환성을 지켜야하기 때문이죠!

그래서 도메인영역이 외부로 노출되지 않도록 경계를 나누는 것입니다. 도메인은 비지니스 영역이기 때문에 변경에 유연하게 대응이 가능해야합니다. 그래야 기능을 추가하고 수정하는데 제약이 없게될거에요!

Copy link
Author

@kdkdhoho kdkdhoho Feb 25, 2023

Choose a reason for hiding this comment

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

무슨 말씀인지 이해했습니다.

그럼, View는 정제된 String만을 받아 출력해주는 식으로 해야 한다면, 그 정제를 어디서 할까? 라는 고민에 대한 제 답은,
비즈니스 로직을 처리해주는 Service에서 하는 것이 적절하다고 생각이 되었어요.
하지만, Service 단에 위치 시킨다고 해서 도메인이 변경되었을 때 유연하게 대응이 가능할까? 라는 생각에는 그렇지 않고 View에서 정제하는 것과 같다는 생각이 들었습니다.

그래서 기존처럼 도메인이 자체적으로 asString이나 toString 같은 함수로 값을 정제해서 건네줘야 하나? 라는 생각도 듭니다.

이 두 가지 방법이 상충관계인 걸까요…?

추가로, 웹 서비스를 개발한다면 DTO를 사용해서 위 문제가 해결 될 것이라고 생각합니다.
하지만 지금은 소규모 단일 콘솔 프로그램이다 보니, 최대한 패키지 구조를 가볍게 가져가고 싶습니다.

Copy link

@joseph415 joseph415 Feb 26, 2023

Choose a reason for hiding this comment

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

저는 dto 사용, 서비스를 만들어서 처리하라는 걸 의도한건 아니구요, 컨트롤러에서 적절하게 의존을 끊어서 view로 보내는 방향을 생각했습니다.

100% 정제하지 않더라도 player들을 List<String> 으로 그냥 View로 전달할 수 있고, Line 도 List<String>으로 전달할 수 있다고 생각했는데 제가 놓치고 있는건지 잘 모르겠네요ㅎㅎ. 저렇게 low한 값을 던저주고 View에서 처리를 할 수 있겠다 생각했습니다~
도메인지식이 도기가 더 높기때문에, 가능한 부분인지 제가 확신은 안서네요🥲

그리고 저는 서비스에서는 dto를 리턴하면 다른곳에서 해당 서비스를 사용할 때 제약이 생기게 되어서 도메인을 리턴하는게 맞다고 생각해요. 그리고 도기가 생각하는 컨트롤러의 역할은 무엇이고, 어디까지인지 궁금해요!

Copy link
Author

Choose a reason for hiding this comment

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

아! 도메인 자체(Player)가 view로 그대로 나가기보다는, Controller에서 도메인으로부터 데이터(List<Player>)를 받아 원시 데이터( List<String>)로만 변환해주고, view에서는 받은 데이터로 원하는 출력 형태로 렌더링 해주는 식으로 하라는 말씀이시군요!!
이렇게하면, 도메인 데이터가 view로 온전히 전달되지 않으면서, view는 사용자가 원하는 방식으로 렌더링 해주기에, 관심사의 분리가 적절히 일어나겠네요..! 왜 이 방법을 생각 못했을까요 😂

제가 생각하는 컨트롤러의 역할은, 단지 View와 Model의 중간 다리 역할로만 생각하고 있었습니다.
따라서 View로부터 사용자의 입력을 받고, 입력값을 Model에 전달해주고, Model에서는 도메인 로직을 수행 후 결과를 Controller에게 반환하면, 값을 받은 컨트롤러는 그대로 View에게 전달만 해주는 역할로 생각하고 있었습니다.
그 외에 렌더링이나 도메인 로직이 아닌 비즈니스 로직Service 단이 해주는 것으로 생각하고 있었어요.
하지만, 지금처럼 단순한 구조를 가져간다면 Controller가 형변환 정도는 해줄 수 있다고 생각이 되네요.

혹시 제가 잘못 생각한 부분이 있을까요?

Copy link

@joseph415 joseph415 Feb 26, 2023

Choose a reason for hiding this comment

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

잘 알고계신 것 같네요ㅎㅎ

그 외에 렌더링이나 도메인 로직이 아닌 비즈니스 로직은 Service 단이 해주는 것으로 생각하고 있었어요.

도메인 로직이랑 비지니스 로직은 동일한 말입니다.

하지만, 지금처럼 단순한 구조를 가져간다면 Controller가 형변환 정도는 해줄 수 있다고 생각이 되네요.

저는 단순한 구조여서 컨트롤러가 형변환 정도를 해주는 것이 아니라, 그게 컨트롤러의 역할이라고 생각해요.
컨트롤러는 다음과 같은 역할을 하는데요

  1. 사용자의 요청을 해석
  2. 어떤 비지니스 로직을 수행할지 결정
  3. 실행결과를 알맞은형식으로 응답

이때 실행결과를 도메인의 의존을 끊기 위해 알맞게 정제해서 응답으로 보냅니다. 따라서 그 자체가 컨트롤러의 역할이 된다고 생각해요ㅎㅎ 아직 서비스까지 생각할 단계는 아니니, 지금처럼 수정해주신 방향이 좋아보입니다ㅎㅎ 고생하셨어요!

지금까지 제가 너무 어렵게 내용과 같이 이야기한게 아닌가 걱정이 되는데,, 도기가 잘 필터링해주시면서 들어주셨을거라 믿어요ㅎㅎ
만약에 이해안가거나 하는것이 있다면 추가질문주세요~!

Copy link
Author

@kdkdhoho kdkdhoho Feb 27, 2023

Choose a reason for hiding this comment

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

도메인 로직과 비즈니스 로직은 동일한 말입니다.

조영호님의 애플리케이션 아키텍쳐와 객체지향 강의에서, Service Layer의 역할은 도메인 로직을 수행하기 위한 준비작업 or 도메인 로직 수행 결과 처리 or 도메인 로직 수행 중 발생한 예외에 대한 후처리 즉, 애플리케이션 로직을 수행하는 것이다. 라고 파악했습니다. 혹시 제가 도메인 로직과 비즈니스 로직(애플리케이션 로직)을 잘못 파악하고 있었나요..?

그렇다면, Service 계층은 어떤 역할을 가지고 있나요??🤔

제가 아직 컨트롤러, 서비스 계층에 대한 분명한 역할을 구분짓지 못하고 있었나봐요 😢
그리고 서브웨이가 말씀해주신 내용 모두 어떤 말씀인지 파악했습니다 ㅎㅎ 알지 못했던 것들을 알려주셔서 감사합니다.

@@ -0,0 +1,47 @@
package ladder.domain.strategy.linestrategy;

Choose a reason for hiding this comment

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

package 위치가 해당 위치가 맞나요~?

);
}

@ParameterizedTest

Choose a reason for hiding this comment

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

ParameterizedTest 👍

# 미션을 진행하며 궁금한 사항
1. 실패하는 테스트가 있어면 커밋을 해선 안된다고 들었는데, TDD의 경우 실패하는 테스트를 만들어야 한다고 강의에서 배웠습니다. 이때, 실패하는 테스트를 구현한 뒤 커밋을 할지, 아니면 전체 기능을 전부 구현한 뒤 커밋할 지 궁금합니다.
-> `test, feat`을 하나로 묶어 커밋하면 실패하는 테스트를 커밋하지 않을 수 있다. 또한 실무에서는 `test-feat-refactor`의 단위보다 하나의 기능 단위로 커밋한다.
2. 현재 생성자를 통해 `Height`와 `Players` 객체를 받고, 각 객체에서 `get()` 으로 값을 꺼내와, 입력받은 높이가 올바른 범위(`players <= height && height <= players * 2`)인지 검증하는 코드인데요. `LadderFactory`가 `Height`와 `Players`를 알고 있지만, 직접 값을 꺼내오는 것이 맘에 들지 않았습니다. 따라서 생성자에서 원시값으로 값을 받아 검증 후에 객체를 생성하는 방식 (`this.height = new Height(height)`) 으로 수정을 하려고 하는데요. 외부에서 원시값을 받고 도메인 내부에서 객체를 생성해도 괜찮은 지 여쭤보고 싶습니다.
Copy link

@joseph415 joseph415 Feb 23, 2023

Choose a reason for hiding this comment

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

3,4번 답변과 동일한데요. 단순히 get하는 행위만 한다면, 큰차이 없다고 생각합니다. 그래서 슬랙으로 2가지 방식을 다 사용한다고 말씀드렸어요.


저 같은 경우에는 객체를 넣을 때도 있고, 간단한 경우에는 그냥 원시값을 넣을때도 있습니다ㅎㅎ
또는, 객체 생성시에 어떤 로직에 의해 원시값이 필요할 때, 파라미터가 많지 않고 객체가 생성이 종속되는 객체의 생성까지 담당하는 것을 표현해주고 싶을 때, 원시값으로 사용하는 것 같습니다.

ex) Orderer는 Order가 생성되야만 생성됨

public static Order from(name, age, orderItem) {
    orderer = new Orderer(name, age)
    
    return Order(orderer, orderItem)
}

그러면, 다른 사람들은 해당 객체의 생성자를 보고, 아 이 객체는 이 객체가 생성되어야지만, 만들어질 수 있구나 라는 것을 알려주는 하나의 방식이 될 수 있다고 생각해요ㅎㅎ.

결론은,,상황에 맞게 하면 될 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. 파라미터가 많아지면 관련있는 파라미터들을 하나의 객체로 만든다는 생각을 못했던 것 같아요.

서브웨이는 상황에 따라 2가지 방법 모두 사용한다고 하셨는데, 덕분에 제가 내린 결론은 이렇습니다.
도메인 간 협력을 할 때에는 default로, 객체 자체를 넘겨주는 식으로 할 것입니다.
하지만, 때에 따라 위 방법이 불필요하거나 원시값이 전달되어야 하는 상황이 온다면, 원시값을 건네주는 식으로 설계를 해봐야겠어요!

3. `Results`클래스의 `findResult` 함수를 설계할 때의 고민입니다.
**함수 파라미터로서 `외부에서 값 자체를 받을지?` 혹은, `객체 자체를 받아서 값을 물어보는 식으로 구현할지` 고민됩니다.**
고민하게 된 이유는 다음과 같습니다. 만약, 미래에 Ladder가 가지는 상태가 변경된다면, 해당 함수도 변경해야 하는 수고로움이 있기에, 이러한 점에서는 Ladder 객체를 받는 것이 더 좋다고 생각됩니다.
하지만, 만약 `(String name, int position, ...)` 같이 값 자체를 받게 된다면, 도메인 간 협력을 한다는 느낌을 받지 못하고, 때에 따라 파라미터 수가 3개를 초과할 가능성도 있다고 생각합니다.
Copy link

@joseph415 joseph415 Feb 23, 2023

Choose a reason for hiding this comment

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

도메인간 협력할 수 있는 부분은 협력해서 서로 메시지를 주고받으면서 의사소통하는 것이 좋다고 생각합니다.

추가적으로 쫌 더 말씀드리자면, 여려 상황이 있을 것 같은데요.

보통 함수의 매개변수가 많아지면, 개념적으로 묶일 수 있는 것들이 있게될겁니다.

ex)

public Order initOrder(String name,int age,String email, List orderItemList, ....) {
...
}

위의 코드에서 주문이란 Order객체를 만드는 펙터리 메서드 인데요, 잘보면 name, age, email은 주문자의 정보들을 의미하죠.
그러면 위의 3개의 파라미터가 Orderer이란 개념으로 묶이게 될겁니다.

fun initOrder(orderer, orderItemList, ...) {
...
}

파라미터 개수가 적어지고, 개념적으로 묶이게 되니, 이해도가 올라갈 겁니다. 결국에, 도기가 말한것처럼 하나의 개념으로 묶이게 되니 응집도가 올라가고, 캡슐화가 되어서 해당 부분만 수정하면되어서 변경에 빠르게 대응이 가능할 수도 있을 것 같아요.
저런 개념으로 묶여서 할 수 있다면, 원시값보다 객체를 보내주는 것이 좋다고 생각합니다.

근데, 파라미터 개수가 그렇게 많지 않고, 저렇게 여러개가 하나의 개념으로 묶이는 상황이 아니라던가?, 내부에서 get 하는 행위만 한다면?,, 원시값과 객체 큰차이가 없을 것 같아요ㅎ

고민하게 된 이유는 다음과 같습니다. 만약, 미래에 Ladder가 가지는 상태가 변경된다면, 해당 함수도 변경해야 하는 수고로움이 있기에, 이러한 점에서는 Ladder 객체를 받는 것이 더 좋다고 생각됩니다.
하지만, 만약 `(String name, int position, ...)` 같이 값 자체를 받게 된다면, 도메인 간 협력을 한다는 느낌을 받지 못하고, 때에 따라 파라미터 수가 3개를 초과할 가능성도 있다고 생각합니다.
4. 3번과 이어지는 질문입니다. 다른 클래스는 모르겠지만 적어도 도메인 객체 내의 함수는 **모두 파라미터로서 원시값보다 객체를 받도록 통일시키는 것**이 코드를 이해하는 데도 좋고, 도메인 객체 간 협력이 잘 이루어진다고 생각합니다.
파라미터로 원시값을 받는 것과 객체를 받는 것에 어떠한 차이가 있나요?

Choose a reason for hiding this comment

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

도메인 객체를 주고 받을 수 있는 상황이면 이렇게 하는것이 좋을 것 같습니다. 근데 내부에서 그냥 get하는 행위만 한다면, 2가지 방식이 차이가 없다고 생각해요.

즉, 비지니스 로직을 처리하기 위함이라면 3번에 답변처럼 서로 협력하는것이 좋은 방향인것같아요.

Copy link
Author

@kdkdhoho kdkdhoho Feb 24, 2023

Choose a reason for hiding this comment

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

고민에 대한 제 기준이 생긴 것 같아요. 감사합니다 ☺️

- Player의 equals 메서드를 이용해 Players에서 동등성 비교하는 방식에서, Player에게 물어보는 방식으로 변경
- Main 함수에서 데이터를 정제하여 View에게 전달하는 방식으로 변경
…형으로 변환해주어 View에게 전달. View에서는 사용자에게 보여주고 싶은 형태로 렌더링 후 출력하는 식으로 리팩터링

+ Application 파일의 라인이 100줄을 넘지 않도록 static 메서드 사용 줄임
Copy link

@joseph415 joseph415 left a comment

Choose a reason for hiding this comment

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

안녕하세요 도기!
리뷰와 전체적인 리펙토링이 잘 반영되어서 코드가 많이 깔끔해진것 같아요! 💯

마지막으로 약간의 코멘트 추가로 남겼는데 확인한번 부탁드릴게요~!

System.out.print(System.lineSeparator() + "실행 결과" + System.lineSeparator());
StringBuilder stringBuilder = new StringBuilder();
for (int i = 0; i < players.size(); i++) {
stringBuilder.append(players.get(i) + " : " + allResult.get(i) + System.lineSeparator());

Choose a reason for hiding this comment

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

string 의 + 연산과 for문 때문에, stringBuilder 로 append 를 써주신것 같은데 내부에서는 그대로 + 를 쓰고있어서 의도하신게 맞는지 궁금합니다.
전체를 append chaining 하게 바꾸도록 해야할 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

아! 말씀의 의도를 파악했습니다!
String의 효율적인 연산을 위해 StringBuilder를 사용했는데, 정작 append() 내부에서는 + 연산을 통해 비효율적이게 하고 있었네요..
감사합니다 😂

ladderFactory = new LadderFactory(height, players, lineStrategy);
ladder = ladderFactory.makeLadder();

OutputView.printLadder(playersToString(players.getPlayers()),

Choose a reason for hiding this comment

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

들여쓰기를

OutputView.printLadder(
        playersToString(players.getPlayers()),
        ladderToString(ladder.getLines()), 
        resultsToString(results.getResults())
);

요렇게 사용하면 쫌 더 보기 편할 것 같아요ㅎㅎ

}
}

private static boolean isQuit(String input) {

Choose a reason for hiding this comment

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

이 부분, 제가 이전 리뷰에서 놓친것 같은데요.
'q'를 입력하면 에러가 납니다.

return PROCEED;
}

private static boolean isAll(boolean proceed, String input) {

Choose a reason for hiding this comment

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

isAll은 2가지 역할을 하고 있는것 같아요(프린트와 전체인지를 판단). 요거 분리를해주는게 어떤가요

LadderFactory ladderFactory = new LadderFactory(height, players, lineStrategy);
Ladder ladder = ladderFactory.makeLadder();
OutputView.printResult(players.asString(), ladder.asString());
private static void isSingleResult(boolean proceed, String playerName) {

Choose a reason for hiding this comment

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

isSingleResult 는 프린트하는 함수인데, 보통 is라는 네이밍은 boolean을 리턴하는 함수에 붙이는 컨벤션입니다. 다른 네이밍을 쓰는게 맞을 것 같아요.

src/main/java/ladder/Application.java Show resolved Hide resolved
Copy link

@joseph415 joseph415 left a comment

Choose a reason for hiding this comment

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

안녕하세요 도기! 마지막 리뷰까지 반영 잘 해주셨습니다~!
리펙토링 해주니, 흐름이 더 잘 읽히는거 같아 좋네요ㅎㅎ
사다리 구현해주시느라 고생 많으셨습니다! 미션은 이만 merge 하겠습니다! 고생하셨습니다💯

@joseph415 joseph415 merged commit 2f5eedd into woowacourse:kdkdhoho Feb 27, 2023
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