-
Notifications
You must be signed in to change notification settings - Fork 53
[4,5단계-사다리] 이지윤 미션 제출합니다. #73
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
Conversation
- 입력받은 플레이어의 결과와 전체 결과를 볼 수 있게 수정하였습니다. - 전체 결과를 출력하기 전까지 확인하고 싶은 플레이어의 결과를 계속 확인할 수 있게 하였습니다.
지윤님 마지막 사다리 미션 고생 많았습니다! |
@@ -9,14 +9,26 @@ public final class InputView { | |||
private InputView() { | |||
} | |||
|
|||
public static String inputLadderWidth() { | |||
System.out.println("사다리의 넓이는 몇 인가요?"); | |||
public static String inputPlayerNames() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정적 메소드를 사용하는 경우가 보통 출력문같은 단순 기능만 나타낼 때 사용하시는 건가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음, 단순 출력뿐 아니라 정적 메서드는 인스턴스 상태에 의존하지 않는 유틸리티성 클래스에서도 자주 사용해요.
InputView나 OutputView에서는 메서드가 특정 객체의 상태를 사용하지 않고 독립적으로 동작하기 때문에 정적 메서드로 구현했어요.
물론 선택적인 부분인데, 객체의 책임과 무관한 보조 기능이거나 재사용 가능한 공통 기능이라면 정적 메서드로 분리하는 것도 하나의 기준이 될 수 있다고 생각합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
구조화도 잘 해두셔서 코드가 잘 읽혔네요 👍
몇가지 코멘트 남겼으니 확인 부탁드려요~~
함수형 구조가 깔끔하긴 했지만, 혹시 재귀 호출을 이렇게 쓰는 방식이 실무에서는 피해야 할 케이스가 있을까요?
현재 구성상 크게 문제될건 없어보입니다 ㅎㅎ
private final List<LadderResult> ladderResults; | ||
private final Map<String, LadderResult> ladderResultCache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
같은 정보를 저장하는것 같은데요! 두가지를 나눈 이유가 있을까요?
Cache는 어떤 역할을 하나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
역할을 분리해서 사용하기 위해서 두 자료 구조를 사용해봤어요.
List
는 데이터를 입력 순서대로 저장하고 전체를 순회하거나 출력할 때 사용됩니다.Map
은 특정 이름으로 빠르게 조회하기 위해 사용했습니다.
→List
를 매번 순회하지 않고 바로 접근할 수 있어서, 조회 성능이 더 좋아진다고 판단했습니다!
같은 데이터를 담고 있더라도 저장과 조회 책임을 분리해서 각각의 장점을 활용한 구조라고 생각했습니다.
혹시 과도한 책임 분리일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중복되는 역할이 생겼다고 느껴지네요
LadderResult 리스트가 필요하다면 Map의 value들을 추출하면 될것 같은데요! Map만 있어도 괜찮지 않을까요?
→ List를 매번 순회하지 않고 바로 접근할 수 있어서, 조회 성능이 더 좋아진다고 판단했습니다!
성능 이야기가 나와서 궁금한 내용입니다!
왜 Map은 List보다 조회 성능이 더 좋은가요? 실제로 List 순회와 비교했을 때 얼마만큼의 성능 차이가 있었나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜 Map은 List보다 조회 성능이 더 좋은가요?
Map에서는 key를 이용해서 내부 해시 구조에서 찾아내는데 O(1)의 시간복잡도를 가져요. 하지만 List에서는 플레이어 수가 많아질수록 순회를 더 많이 해야하니까 O(n) 시간복잡도를 가지게 돼요. 그래서 조회가 빈번하게 일어나니까 Map으로 캐싱을 해줘야겠다!
하고 생각하게 되었어요.
실제로 List 순회와 비교했을 때 얼마만큼의 성능 차이가 있었나요?
구현을 할 당시에는 List와 Map의 성능 차이를 확인하지 않았어요... 그래서 제가 정해둔 최대 Player 수인 24로 정해놓고 List와 Map의 탐색 시간을 측정해보니 의미있는 성능 차이가 나지 않더라구요... 😣
그래서 어느 정도의 수까지 가야지 차이가 나는지도 확인해봤어요.
100,000
정도는 되어야지 그래도 좀 눈에 띄는 성능이 차이 나더라구요,,

저번에도 비슷한 리뷰를 받았던 것 같은데,, 또 체크를 못했네요ㅠ
중복된 역할을 사용한 게 맞는 거 같아요! 좋은 지적 감사합니다! 수정해볼게요 🥲
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍👍
성능이라는 이야기를 할 때는 정확한 지표로 표현할 수 있는지 한번 생각해보면 좋을 것 같아요!
public record LadderResult( | ||
String player, | ||
String result | ||
) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Player, Result 객체를 가지지 않고 String으로 필드를 둔 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
처음엔 LadderResultBoard.of()
에서 조립을 하기 때문에 LadderResult
가 Player, Result 객체를 직접 들고 있어도 괜찮지 않을까 고민했어요.
그런데 로직을 정리하다 보니 LadderResult
는 도메인 행위 없이 최종 결과를 표현하기 위한 역할이라서 굳이 도메인 객체를 직접 들고 있을 필요가 있을까?
하고 생각이 들었습니다.
그래서 도메인 의존성을 줄이고 필요한 데이터만 전달하는 걸 고려해서 String 필드로 구성했어요.
혹시 더 나은 구조가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그런데 로직을 정리하다 보니 LadderResult는 도메인 행위 없이 최종 결과를 표현하기 위한 역할이라서 굳이 도메인 객체를 직접 들고 있을 필요가 있을까? 하고 생각이 들었습니다.
이 의견에서 봤을 때 LadderResult는 DTO 객체로 보이는데요?
현재 패키지 구조상으로 봤을 때는 domain이라고 지칭하고있는 것 같아요
지윤님은 보통 Dto를 패키지 상 어디에 위치하시는 편일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지윤님은 보통 Dto를 패키지 상 어디에 위치하시는 편일까요?
저는 전달받은 데이터를 도메인 객체로 변환하는 책임이 DTO에 포함되는 경우가 많다고 생각해서, DTO를 도메인과 가장 밀접한 위치에 두는 게 자연스럽다고 생각했어요.
하지만 일반적으로는 controller 하위 패키지에 위치하는 경우가 많다고 하더라구요,,
사실 이번에 구현에 집중하느라 패키지 구조까지 신경쓰지 못한 것 같아요.
주노의 리뷰를 받고 생각해보니 아직 어디에 위치하는 게 나을지에 대한 기준
을 명확히 정하지 못한 것 같습니다 🥺
LadderResult
는 말씀해 주신 것처럼 DTO 객체가 맞아요!
제가 domain.ladder.result
패키지로 구성한 이유는 플레이어와 결과의 매핑이 직접 도메인 로직을 포함하지는 않지만 흐름을 가진다고 생각해서 사다리 도메인과 가깝다고 느꼈기 때문입니다.
혹시 어떤 기준으로 dto의 패키지 구조를 정하시는지 주노의 의견을 듣고 싶습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신 내용에 동감합니다~!
제가 생각하는 구조로는 view 레벨의 영역에서 표현되는 객체이니 view와 관련된 패키지에 둘것 같네요 👍
4,5단계까지 오느라 정말 고생 많으셨어요! 마지막까지 화이팅 입니다~~~ 😊 |
import java.util.List; | ||
import java.util.stream.Stream; | ||
|
||
public record RequestLadderGame( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Choi-JJunho
RequestLadderGame 하나에 모든 입력값을 한 번에 받고 있는데 이 부분에 대해서는 어떻게 생각하시는 지 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞아요.. 저도 그 부분이 좀 신경 쓰이더라구요!
다음엔 분리해서 빠르게 알 수 있게 해야겠어요 감사합니다 😆
- 기존 List와 Map을 이용해 결과를 저장하였는데, 중복되는 저장 방식으로 Map만을 사용하게 변경하였습니다.
안녕하세요 주노 🍀
사다리 미션 정말 어렵네요 🥺 마지막까지 리뷰 잘 부탁드립니다 !
🤔 고민한 부분
DTO를 어떻게 구성해야 할지
처음엔 이름, 실행 결과, 사다리 높이 등을 각각 다른 DTO로 받아보려 했어요.
그런데 그렇게 하다 보니 입력값 검증(빈 값 등) 로직이 여러 곳에 흩어져 중복되고 컨트롤러에서 조합해 도메인을 생성하는 흐름도 복잡하고 책임이 분산되는 느낌이었어요.
그래서 지금은
RequestLadderGame
하나에 모든 입력값을 한 번에 받고, 이 DTO 내부에서 필요한 도메인 객체들을 생성하는 방식으로 구현했어요.입력 관련 책임이 한 곳으로 모이고 흐름이 단순해졌지만, 반대로
입력 단위를 나누는 게 더 유연한 구조일 수도 있지 않을까?
하는 생각도 여전히 들더라고요.재입력 로직을 어떻게 구성할지
"결과를 보고 싶은 사람"을 입력받는 과정에서 잘못된 이름을 입력하면 다시 재입력받아야 했는데,
while 문
을 사용하면 indent가 2 이상이 되어버리더라구요.try-catch
도 고려했지만, 책임을 명확하게 분리하면서 재사용도 가능한 구조로 만들고 싶어 고민하다가Supplier
와Function
를 받아 함수형 재귀로 구현해봤습니다!🙋🏻♀️ 궁금한 부분