Step3 리뷰요청드립니다 #4230
Step3 리뷰요청드립니다 #4230
Conversation
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
3단계 미션 진행하느라 수고했어요. 👍
변경을 최소화하기 위해 노력했는데 생각보다 쉽지 않죠?
아직까지 연습이 되지 않아 그러니 너무 조급해하지 마세요.
원시값을 포장하지 않아 원시값 포장에 대한 피드백 남겼으니 피드백 반영해 보면 점진적인 리팩터링 도전해 보세요.
원시값 포장에 따른 변경이 크기 때문에 생각보다 점진적인 리팩터링이 쉽지 않을 겁니다.
Q1. 개발은 Rank에 2등을 추가하는 작업부터 시작하였습니다.
Rank를 수정하고, Rank가 호출하는 LottoResult -> Lottos 순으로 접근해서 Lotto까지 접근하는 과정으로 시도해보았습니다.
시도하다보니 Lotto로 생성하던 winningLotto를 분리해서 WinningLotto를 만들었습니다.
A1. 순서는 그리 나쁘지 않았다 생각해요. 기능 추가/변경을 위한 작업을 할 때 순서는 가능하면 어떤 객체와도 의존 관계를 가지고 있지 않은 마지막 노드(보통 leaf node라고 함)부터 접근하면 변경을 최소화할 수 있습니다.
예를 들어 지금의 경우 Rank, WinningLotto 추가, Lottos가 WinningLotto 사용하도록 변경 등의 순으로 진행하면 될 것 같아요.
| public class Lotto { | ||
| public static final int LOTTO_NUMBER_SIZE = 6; | ||
|
|
||
| private final Set<Integer> numbers; |
There was a problem hiding this comment.
Set 콜렉션 내부의 Integer도 원시값이다.
이 원시값을 포장해 보면 어떨까?
| } | ||
|
|
||
| public LottoResult check(Lotto winningLotto) { | ||
| public LottoResult check(WinningLotto winningLotto) { |
There was a problem hiding this comment.
check 메서드 이름이 메서드 의도를 잘 드러내지 않는 느낌이 듦
AI에게 피드백을 받아보면 어떨까?
| int match = winningLotto.match(lotto); | ||
| boolean bonusMatch = winningLotto.isBonusMatch(lotto); | ||
| result.addMatch(match, bonusMatch); |
There was a problem hiding this comment.
| int match = winningLotto.match(lotto); | |
| boolean bonusMatch = winningLotto.isBonusMatch(lotto); | |
| result.addMatch(match, bonusMatch); | |
| Rank rank = winningLotto.match(lotto); | |
| result.addMatch(rank); |
WinningLotto를 분리한 만큼 이 객체에 등수를 구하는 책임을 위임하면 어떨까?
정보 전문가에게 책임을 부여
정보 전문가 (Information Expert)
어떤 작업을 수행하는 데 필요한 정보를 가장 많이 가지고 있는 객체에게 그 작업을 수행할 책임을 부여함
예시: 로또 구매 가능 개수를 아는 책임은 보유 금액(Money) 정보를 가진 객체에게 부여
| @@ -0,0 +1,34 @@ | |||
| package lotto.domain; | |||
|
|
|||
| public class WinningLotto { | |||
|
|
||
| public class WinningLotto { | ||
| private final Lotto lotto; | ||
| private final int bonusNumber; |
There was a problem hiding this comment.
"규칙 3: 모든 원시값과 문자열을 포장한다." 원칙 적용
| private final Lotto lotto; | ||
| private final int bonusNumber; | ||
|
|
||
| public WinningLotto(Lotto lotto, int bonusNumber) { |
There was a problem hiding this comment.
주 생성자는 생성자의 마지막에 구현하는 것이 관례
|
피드백 적용해보았습니다. 말씀주신 check()는 지금보니 이름이 모호한거 같아, 당첨번호를 찾는 findWinner()로 수정해보았습니다. 피드백주시면 한번 더 체크해보겠습니다! 그리고 마지막으로, 사소한 부분이긴한데 점검하다보니 |
2 participants
안녕하세요!
로또 수동 작성해보았습니다!
하나를 수정하기 시작하니 계속 빨간불이 떠서, 이번에도 커밋을 쪼개지 못한거 같습니다.
위 경우에는 어떻게 커밋하는 습관이 좋을까요?
개발은 Rank에 2등을 추가하는 작업부터 시작하였습니다.
Rank를 수정하고, Rank가 호출하는 LottoResult -> Lottos 순으로 접근해서 Lotto까지 접근하는 과정으로 시도해보았습니다.
시도하다보니 Lotto로 생성하던 winningLotto를 분리해서 WinningLotto를 만들었습니다.
이렇게 시도하는 과정이 괜찮을까요?
변경을 최소하하면서 개발해보라고 하셨는데, 그러지 못한거 같네요ㅠ
개발하다보니, Lotto의 Set의 Integer를 포장했어야하는 생각이 드는데요, 이 부분은 한번 적용해보겠습니다ㅠ
피드백 주시면 확인해보겠습니다!
완전 다른 시각이 존재하면, 다시 적용해보는것도 좋을거 같습니다