[2주차] 로또 - 클린 코드 #68
[2주차] 로또 - 클린 코드 #68
Conversation
There was a problem hiding this comment.
리드미를 작성해주시면 좋을 것 같아요!
리뷰할 때 어떤 부분을 중점적으로 봐야하는지 적혀있으면 정말 큰 도움이 될 것 같고, 프로젝트 설명을 해주시면 도움이 될 것 같아요!
| import static domain.Lotto.multipleLottoGenerator; | ||
|
|
||
| public class LottoController { | ||
| public static void main(String[] args) { |
There was a problem hiding this comment.
main 함수가 전체적으로 조금 긴 느낌이 드는데요 적절한 기준에 따라서 나눠주셔도 좋을 것 같아요!
| public static void main(String[] args) { | ||
| int totalInvestment = InputView.inputToBuyLotto(); // 구입금액을 입력해주세요. (입력) | ||
| int numberOfManualLottos = InputView.inputToBuyManualLotto(); // 수동으로 구매할 로또 수를 입력해주세요. (입력) | ||
| int numberOfAutoLottos = totalInvestment / 1000 - numberOfManualLottos; |
There was a problem hiding this comment.
요 로직은 컨트롤러에 있는 것 보다 다른 방향으로 보내봐도 좋을 것 같은데 어떻게 생각하시나요?
최대한 비즈니스와 관련된 로직은 컨트롤러에 없는 것이 유지보수하기 편할 것 같아요!
There was a problem hiding this comment.
고생 많으셨습니다!
큰 프로젝트에서는 방향을 잃어버리는 경우가 당연히 정말 많은데요
저는 처음에 시작할 때 어떤어떤 어떤 기능을 만들어야하는 것인지를 명확하게 적어두고
intellij 에 bookmark (f3, command+f3) 기능을 활용하곤 합니다!
네이밍을 할 때 numberBuffer 와 같은 네이밍은 최대한 지양하려고 해요
나중에 이게 무슨 number 가 들어오는지 헷갈릴테니까요! (최대한 구체적인 사용하는 목적에 따라 두려고 합니다)
아직 테스트코드가 없는 것 같아서 이번에는 테스트코드를 추가해보시면 어떠실까요?
| } | ||
| } | ||
|
|
||
| public LottoDTO countCollectNumber(List<List<Integer>> lottoCollection, List<Integer> lastWeekWinningNumbers, int bonusWinningNumber, int totalInvestment) { |
There was a problem hiding this comment.
현재 이 클래스는 필드 변수가 사실상 0개만 들어있는 클래스인데요
객체지향을 연습하시는 과정이니 이 클래스를 적당한 클래스로 바꿔보실 수 있으실까요?
예를 들면 상금에 대한 클래스나 로또에 이 계산을 넘겨도 좋을 것 같고요
https://www.mimul.com/blog/oop-alternative-to-utility-classes/
| for (List<Integer> lotto : lottoCollection) { | ||
| int matchCount = 0; | ||
| boolean bonusMatch = false; | ||
|
|
||
| for (int number : lotto) { | ||
| if (lastWeekWinningNumbers.contains(number)) { | ||
| matchCount++; | ||
| } | ||
| if (bonusWinningNumber == number) { | ||
| bonusMatch = true; | ||
| } | ||
| } | ||
| matchCountMap.put(String.valueOf(matchCount), matchCountMap.get(String.valueOf(matchCount)) + 1); | ||
| if (matchCount == 5 && bonusMatch) { | ||
| matchCountMap.put("5+Bonus", matchCountMap.getOrDefault("5+Bonus", 0) + 1); | ||
| } | ||
| } |
| public class Lotto { | ||
|
|
||
| private static List<Integer> autoLottoGenerator() { | ||
| final int RANDOM_VALUE_MAX = 45; |
There was a problem hiding this comment.
이런 상수는 스태틱 변수로 두는 것이 좋을 것 같아요!
혹시 왜 여기만 final 이 붙어있을까요?
|
|
||
| public class Lotto { | ||
|
|
||
| private static List<Integer> autoLottoGenerator() { |
There was a problem hiding this comment.
객체를 만드는 생성의 과정과
객체를 사용하는 객체의 기능을 분리해봐도 좋을 것 같아요!
https://jusungpark.tistory.com/14
| @@ -0,0 +1,15 @@ | |||
| package dto; | |||
|
|
|||
| public class LottoDTO { | |||
| public Integer correct3, correct4, correct5, correct5WithBonus, correct6; | ||
| public Double income; | ||
|
|
||
| public LottoDTO(Integer correct3, Integer correct4, Integer correct5, Integer correct5WithBonus, Integer correct6, Double income) { |
There was a problem hiding this comment.
현재와 같이 필드 수가 늘어나면 나중에 확인하기 힘들어 질 것 같아요
생성자에는 4개 이상의 필드가 있으면 실수할 확률이 많이 늘어난다고 해요
이 필드를 줄여보면 어떨까요?
|
|
||
| public class InputView { | ||
|
|
||
| private static final Scanner scanner = new Scanner(System.in); |
There was a problem hiding this comment.
Scanner.close 를 호출하지 않으면 memory leak 이 발생할 수 있어서 잘 close 할 방법을 찾아봐도 좋을 것 같아요
안녕하세요. 은우님! 이번에 리뷰를 받게 된 신지훈 입니다.
지난 미션과 다르게 이번 미션은 미션의 완성보다 처음부터 혼자 만들어 가는 것에 초점을 두어 미션을 진행했습니다. 아직 자바 경험이 너무 없다보니 코드가 엉망이고 지키지 못한 요구사항들이 많습니다. 기능 구현은 모두 했지만 시간 관리를 못해서 리팩토링과 테스트 코드 작성은 진행을 못했습니다..ㅜ 리뷰를 진행하며 리팩토링과 테스트 코드 작성 또한 혼자 힘으로 한 번 해보고 싶습니다. 리팩토링과 테스트 코드를 작성할 때 신경 썼으면 하는 부분도 리뷰 때 짚어주시면 성실히 해보겠습니다!!
리뷰와 더불어 가장 궁금(?)한 내용이기도 하고 스스로 코드를 짜며 어이가 없던 점이 하나 있는데, 코딩을 하는 시간이 길어지거나 작성한 코드의 양이 꽤 많아졌을 때 제가 작성한 코드들의 흐름을 놓치거나 변수명 까먹는(?) 상황이 많이 발생했는데 개발을 끝마칠 때까지 프로젝트(?)의 흐름을 잊지 않는 법이나 방대한 양의(지금은 작지만,,,ㅎ) 코드를 짤 때 코드 속에서 길을 잃지 않는 팁이 있는지 궁금합니다.
바쁜 시간 중에 리뷰 해주셔서 감사하고 제가 좀 더 구체화 했으면 하는 부분이 있다면 모두 짚어주세요...! 감사합니다..!!!!