[2주차]객체지향 코드 연습(enohs) #30
Conversation
enohs
requested review from
Hoya324,
KoSeonJe and
TaetaetaE01
as code owners
There was a problem hiding this comment.
요구사항에 따른 기능을 구현하고 클래스로 분리하는 것보다, 먼저 설계하는 것이 더 좋습니다.
요구사항을 저희가 쭉 나열했는데, 각 요구사항�에는 어떤 책임들이 있을지 분리하고, 책임을 맡을 클래스가 무엇이 있으면 좋을지 생각해보는 것이 좋습니다.
예를 들어, "로또 구입 금액을 입력하면 구입 금액에 해당하는 만큼 로또를 발행해야 한다." 라는 요구사항이 있을 때,
책임에는 크게 아래 두 가지가 있습니다.
- 로또 구입 금액 입력
- 구입 금액에 해당하는 만큼 로또 생성
그러면 입력을 받는 책임을 가진 클래스, 구입 금액에 해당하는 만큼 로또를 생성하는 책임을 가진 클래스를 생성하면 되는 것입니다.
너무 어렵게 느껴진다면, MVC 패턴을 적용해보면서 객체지향을 이해해도 좋습니다.
에러 체크 추가, 로또 비교 기능 추가
There was a problem hiding this comment.
이전보다 각 클래스에 대한 책임과 역할에 대해 분리하려고 노력하신 부분이 많이 보여서 좋았습니다!
하지만 역할을 나누었지만 한 클래스, 메소드내에 한 번에 많은 로직을 처리하려고 하는 것들이 보입니다.
한 메소드에 20줄이 넘지 않도록 설계하여 분리하는 고민도 해보시는 것도 크게 도움이 될 거라고 생각합니다!
또 static 키워드를 많이 사용하셨는데 어떤 목적을 가지고 사용했는지 궁금합니다! 만약 제가 생각하는 부분에서 어려움이 있었다면, 의존성 주입(new 생성자 생성 시점)에 대해 검색 해보시고 고민해보는 것이 좋을 거 같습니다!
고생 많으셨습니당!~
| public static int price(){ | ||
| System.out.println("구입금액을 입력해 주세요."); | ||
| String lottoPrice = scanner.nextLine(); | ||
| int intPrice = CheckError.isNum(lottoPrice); | ||
| return intPrice; | ||
| } | ||
|
|
||
| public static String winningNums() { | ||
| System.out.println("\n당첨 번호를 입력해 주세요."); | ||
| String winningNums = scanner.nextLine(); | ||
| return winningNums; | ||
| } | ||
|
|
||
| public static int bonusNum(){ | ||
| System.out.println("\n보너스 번호를 입력해 주세요."); | ||
| int bonusNum = scanner.nextInt(); | ||
| CheckError.numRange(bonusNum); | ||
| return bonusNum; | ||
| } |
There was a problem hiding this comment.
input 메소드를 static으로 구현하신 이유가 있으실까요??
There was a problem hiding this comment.
전체적으로 static을 사용한 것은 객체 생성 없이 프로그램을 진행시키기 위함이었습니다. 그런데 static에 대해 알아보니 메모리에서 지워지지 않는 특징이 있더군요!
그래서 남용하면 성능에 영향을 준다는 사실을 알게 되었습니다.
아무래도 static을 어떻게 쓰는 것이 메모리 사용에 있어 효율적일지 생각해봐야 할 것 같습니다
|
|
||
| public class CompareLottos { | ||
|
|
||
| public static Map<String, Integer> compareNums(List<List<Integer>> myLottos, List<Integer> winLotto, int bonusNum) { |
There was a problem hiding this comment.
전체적으로 클래스 네임이 잘못되었습니다.
동사 + 명사로 되어있는데, 행위를 수행하는 주체의 이름을 동사와 함께 나타내는 것은 좋지 않은 것 같습니다. 모두 명사로 바꿔주세요.
ex) StartLotto -> LottoStarter
패키지 명이 모두 두개의 단어로 되어있습니다. 패키지 명은 하나의 단어를 사용하는 것을 지향하고, 대문자를 사용하지 않는 것이 원칙입니다. 모두 수정해주세요.
코드 컨벤션을 전체적으로 신경써야 할것 같고, 매직넘버 그리고 변수명을 더 구체적이고 명확하게 짓도록 고민해주셔야할 것 같습니다.
저장이 필요한 데이터와, 일회용인 데이터를 구분하는 고민도 필요해보입니다. 예를 들어, CalcProfit에, 20000000 1등 상금을 그냥 raw값으로 나타내고 있는데, 이게 코드 상에 직접 적어주고, 메소드 내에 적어도 괜찮은지. 아니면, 이것을 따로 관리해주고 저장하고 있어야하는 지 고민해보세요.
만약 당첨 상금을 변경해야한다면? 본인이 아닌 다른 개발자가 당첨금을 바꾸려고 하는데, CalcProfit에 당첨금이 있는 것을 당연히 못찾지 않을까 싶습니다.
코드를 작성할 때 가장 중요하게 생각해야될 점 중 하나가 협업입니다. 다른 사람이 내 코드를 보고 이해할 수 있는지? 잘 수정할 수 있는지, 생각하면서 개발하는 것이 좋습니다
| int inputPrice = InputNums.price(); | ||
|
|
||
| MakeLottoLists lottoLists = new MakeLottoLists(); | ||
| List<List<Integer>> myLottos = lottoLists.makeInputLottoList(inputPrice); |
There was a problem hiding this comment.
Lotto 객체를 잘 활용하지 못하고 계신 것 같습니다. Lotto 객체를 활용해주세요
There was a problem hiding this comment.
맞습니다. 제가 직접 만들어서 사용해보려고 하다가 활용을 못했습니다 ㅎㅎ 사용하도록 노력해보겠습니다!
|
|
||
| public class CompareLottos { | ||
|
|
||
| public static Map<String, Integer> compareNums(List<List<Integer>> myLottos, List<Integer> winLotto, int bonusNum) { |
There was a problem hiding this comment.
| public static Map<String, Integer> compareNums(List<List<Integer>> myLottos, List<Integer> winLotto, int bonusNum) { | |
| public static Map<String, Integer> compareNums(List<Lotto> myLottos, List<Integer> winLotto, int bonusNum) { |
Lotto객체를 활용하지 못하는 것 같습니다.. Lotto 객체를 활용해주세요
변수명 변경, 상수 사용, 의존성 낮춤
의존성 주입
사용하지 않는 코드라인 제거
Application에서 먼저 구현을 한 뒤 기능에 따라 클래스를 분류하려고 했으나 로또 번호를 비교하기 위한 방법을 고민하다 결국 실력 부족 및 시간 부족으로 완성하지 못했습니다..... 너무 아쉽고 민망하네요ㅠㅜ
피드백 후에 다시 한번 스스로 만들어보는 시간을 가져보도록 하겠습니다.