Conversation
Co-authored-by: 권희준 <2390kwon@g.skku.edu>
Co-authored-by: 권희준 <2390kwon@g.skku.edu>
| public Lotto generateTickets(Integer price) { | ||
| validatePrice(price); | ||
| Integer numberOfTickets = calculateNumberOfTickets(price); | ||
| List<LottoTicket> generatedTickets = new ArrayList<>(); | ||
| for (Integer i = 0; i < numberOfTickets; i++) { | ||
| generatedTickets.add(new LottoTicket(numberListGenerator.generate())); | ||
| } | ||
|
|
||
| return new Lotto(generatedTickets); | ||
| } |
There was a problem hiding this comment.
"모든 원시 값과 문자열을 포장하라"에 대해 아직 완벽히 이해하지 못했습니다.
for문 안에 i도 Integer로 바꿔야 하나요? 전혀 의미가 없어 보이고, IDE에서도 다시 int로 바꾸라고 하더라고요. (일단 적용은 했습니다)
또한, 문자열을 포장한다는 것이 어떤 의미인가요? OutputView의 print문 안의 문자열도 포장해야 하나요?
"모든 원시 값과 문자열을 포장(Wrapping)하라"는 원시 타입(Primitive Type)을 참조 타입(Wrapping class)로 바꾸란 의미가 아닙니다..! 비슷한 용어가 있어서 원시 값을 포장한다는 말에 오해가 생겼네요.🤔
int numberOfTickets = calculateNumberOfTickets(price); 와 같이 바꾸셔도 괜찮습니다!
이 문장의 핵심은 값을 포장한다. 입니다.
Int numberOfTickets 을 예로 들어보자면,
numberOfTickets라는 이름을 가진, int 값 그 자체입니다.
이제 이 값을 객체로 포장하는 것이 "모든 원시 값과 문자열을 포장(Wrapping)하라"의 목표입니다.
LottoTicket 처럼 말이죠.
그럼 값을 왜 포장해야하는가?
numberOfTickets 같은 경우 for문 조건으로만 사용되는 값이니 다른 예시를 들어볼게요.😄
먼저 값을 포장하는 것은 값이 어떤 값인지 보장하기 위해서 입니다.
객체로 감싸면서 타입이 정해지니, 어떤 종류의 값인지 보장할 수 있다는 것이죠.
보장하지 않는다면, 아래와 같은 실수가 일어날 수 있습니다.
int a = 0;
int b = 1000;
generateTickets(b,a);
public Lotto generateTickets(int a, int b) {
// a=b 이고, b=a이다...?
}
인수로 전달된 a와 b 값은 어떤 값인지, 전달받은 generateTickets() 입장에서 알 수가 없죠.
값을 전달할 때마다 a와 b가 정말 a와 b에 해당하는 값인지 검증할 수도 있겠지만, 코드가 정말 많아질 거에요.😅
그래서 Price라는 객체로 포장해서 파라미터를 설정하면, 컴파일 단계에서 오류를 잡는 것입니다.
이제 generateTickets(Price price){} 와 같이 바꾼다면, price에 해당하는 값만 있다는 게 보장되겠네요!🙌
원시 타입을 포장해야 하는 이유는 테코블 게시글에서도 참고해주세요!
위 내용을 참고하여, 현재 코드에서 값 포장을 위한 리펙터링 진행주시면 될 것 같아요!
진행하시면서 그럼 모든 값을 포장해야 할까?와 같은 고민이 생길 수 있어요.
고민하신 내용에 대해 답변으로 남겨주세요. 😄
값을 전달할 때마다 a와 b가 정말 a와 b에 해당하는 값인지 검증할 수도 있겠지만, 코드가 정말 많아질 거에요.😅
사실 이 문제도 완전히 해결된 건 아니거든요.😮
위 고민과 함께 해결할 방향을 고민해보면 좋을 것 같아요.😄
| public Double getProfitRate(LottoResult result, Integer price) { | ||
| Integer totalProfit = 5000 * result.getThreeCorrectCount() | ||
| + 50000 * result.getFourCorrectCount() | ||
| + 1500000 * result.getFiveCorrectCount() | ||
| + 2000000000 * result.getSixCorrectCount(); | ||
|
|
||
| return totalProfit.doubleValue() / price; | ||
| } |
There was a problem hiding this comment.
여기서 5000 과 같은 값은 THREE_CORRECT 등과 달리 전역 변수로 관리하지 않으신 기준이 있으신가요?
또한, OutputView.showLottoStatistics()의 출력 값에서도 같은 값이 사용되고 있습니다.
만약 요구 사항이 변경된다면, 두 곳 모두 코드를 수정해야 할 것 같아요.
이 또한 값을 포장하며 해결할 수 있을 것 같아요.
Enum 클래스를 활용해보아도 좋습니다.😄
| public class LottoResult { | ||
| private final Integer threeCorrectCount; | ||
| private final Integer fourCorrectCount; | ||
| private final Integer fiveCorrectCount; | ||
| private final Integer sixCorrectCount; | ||
|
|
| private void showLottoTicket(LottoTicket lottoTicket) { | ||
| List<Integer> ticket = lottoTicket.getTicket(); | ||
| System.out.println(ticket); | ||
| } |
There was a problem hiding this comment.
public class LottoTicket {
private final List<Integer> ticket;
public List<Integer> getTicket() {
return ticket;
}
}
LottoTicket을 일급 컬랙션으로 잘 감싸주셨습니다.
하지만, getTicket()는 감싸둔 내부 필드를 외부에 그대로 공개하는 것입니다.
List ticket = lottoTicket.getTicket();
만약 LottoTicket의 ticket의 타입이 Set이 된다면?
OutputView에 있는 List<Integer> ticket 도 함께 변경해야 합니다.
또한, OutputView에서 List<Integer> ticket를 그대로 가져왔기에,
LottoTicket의 역할을 List<Integer> ticket이 수행할 수 있습니다.
LottoTicket를 경유하지 않고도, List<Integer> ticket를 사용하여
LottoTicket.getResult()와 같은 작업을 OutputView에서도 할 수 있게 됩니다.
LottoTicket가 책임지고 getResult()를 진행해야 하는데, 아무 곳에서나 할 수 있게 되어버립니다..!
결국, 코드가 여러 곳으로 나뉘어도 찾기 어렵고, 변경할 범위도 넓어지게 될 수 있어요!
때문에, 값을 감싼 객체는 자신의 값을 책임지고 관리할 수 있어야 합니다.🤔
이 점을 고려하여 리펙터링을 진행해보면 좋을 것 같아요.😄
| for (Integer number : winnerTicket.getTicket()) { | ||
| count += Boolean.compare(ticket.contains(number), false); | ||
| } |
| public static void main(String[] args) { | ||
| InputView inputView = new InputView(); | ||
| OutputView outputView = new OutputView(); | ||
| NumberListGenerator numberListGenerator = new RandomNumberListGenerator(TICKET_LENGTH); | ||
| Cashier cashier = new Cashier(numberListGenerator); | ||
|
|
||
| Integer price = inputView.inputPrice(); | ||
| Lotto lotto = cashier.generateTickets(price); | ||
| outputView.showLottoTickets(lotto); | ||
|
|
||
| LottoTicket winnerTicket = inputView.getWinnerTicket(); | ||
| LottoResult result = cashier.getResults(lotto, winnerTicket); | ||
| outputView.showLottoResults(result, cashier.getProfitRate(result, price)); | ||
| } | ||
| } |
There was a problem hiding this comment.
지난 미션의 랜덤값 테스트를 반영하여 NumberListGenerator를 인터페이스 구현체로 두신 것 같아요!🙌
하지만 로또 추첨 전체 과정을 테스트한다면, 현재 인터페이스 구현체를 활용해 출력값을 조절할 수 있을까요?
There was a problem hiding this comment.
요구사항에 없더라도, 가능하면 테스트 코드도 함께 작성해보면 학습에 좋을 것 같아요~😄
지금처럼 코드 리펙터링이 왕왕 일어날 때,
하나의 코드를 수정할을 때, 어디에 그리고 몇 개의 테스트가 깨지는 지로
얼마나 OOP를 만족하는 중인지 셀프 피드백이 가능합니다!🤔
혹은 테스트 짜기 어렵다면, 그것 또한 잘못된 코드란 뜻일 수도 있구요..!
LottoTicket, Lotto, Cashier 간 역할 분배에 대한 고민 해결에도 도움이 될 거에요!😄
| # 1단계 기능 요구사항 | ||
| - [X] 로또 구입 금액을 입력하면 구입 금액에 해당하는 로또를 발급해야 한다. | ||
| - [X] 로또 1장의 가격은 1000원이다. | ||
| - [X] 새로운 프로그래밍 요구사항 |
| for (Integer i = LOWER_BOUND; i < UPPER_BOUND; i++) { | ||
| numberList.add(i); | ||
| } |
There was a problem hiding this comment.
전혀 의미가 없어 보이고, IDE에서도 다시 int로 바꾸라고 하더라고요. (일단 적용은 했습니다)
IDE에서 바꾸라고 하는 이유는 무엇일까요?😄
요구사항이 잘못되었다고 느껴졌을 때, 이를 설득시키는 것도 중요하다 생각합니다.
요구사항 또한, 충분히 잘못될 수도, 바뀔 수도 있으니까요!🤔
비단, 요구사항뿐만 아니라 페어프로그래밍이나 개발자 간 회의에서도 설득해야 할 일이 많습니다.😑
이런 맥락에서 잘못된 이유는 확실히 짚고 넘어가면 좋을 것 같아요!😄
안녕하세요, SCG 최연우입니다.
Java는 아직 익숙하지 않고, Python, C, C++는 조금 할 줄 압니다.
intelliJ IDEA 사용하였습니다.
미션 했을 때 어려웠던 부분
indent 제한 때문에 for 안에 if 못 넣는 것
LottoTicket, Lotto, Cashier 간 역할 분배
질문
"모든 원시 값과 문자열을 포장하라"에 대해 아직 완벽히 이해하지 못했습니다.
for문 안에 i도 Integer로 바꿔야 하나요? 전혀 의미가 없어 보이고, IDE에서도 다시 int로 바꾸라고 하더라고요. (일단 적용은 했습니다)
또한, 문자열을 포장한다는 것이 어떤 의미인가요? OutputView의 print문 안의 문자열도 포장해야 하나요?