Conversation
- 금액 입력 및 유효성 검사 - 로또 자동 생성 및 구매 내역 저장 - 당첨 번호 랜덤 생성 (보너스 포함) - 로또 당첨 결과 분석 및 등수 판별 - 당첨 통계 및 수익률 계산 기능 추가 - [ERROR] 포맷의 기본 에러 메시지 출력 로직 구성
This reverts commit 1f140c6.
|
|
||
| fun checkThousant(input: String, list: List<Int>) { | ||
| println("[ERROR] 1000원 단위로 입력해주세요.") | ||
| } |
There was a problem hiding this comment.
혹시 위 함수 매개변수랑 아래 함수의 lottoList 매개변수는 사용하지 않는데 넣은 이유가 있을까요?
| when (errorType) { | ||
| "size" -> println("[ERROR] 로또 번호는 6개여야 합니다.") | ||
| "duplicate" -> println("[ERROR] 중복 없는 숫자만 입력해야 합니다.") | ||
| "range" -> println("[ERROR] 숫자는 1~45 사이여야 합니다.") |
There was a problem hiding this comment.
size. duplicate, range 를 errorType 이라는 문자열 매개변수로 받는데,
오타 방지, 유지보수, 재사용등을 위해서라도 Enum class(좀 더 추천)나 sealed class로 선언한다면 더 편리할 것 같아요
| if (inputMoney == null) { | ||
| if (!checkInputNull()) return -1 | ||
| continue | ||
| } |
There was a problem hiding this comment.
입력이 null로 받아지는 상황은 어떤 게 있나요?
| if (!checkTestFail()) return -1 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
뭔가 굳이 싶은 조건문이 조금 많아서 복잡해 보입니다.
조금씩만 수정하면 더 깔끔하게 바뀔 것 같은데 한 번 생각해 보시면 좋을 것 같아요
| @@ -0,0 +1,16 @@ | |||
| package lotto.lottoMachine | |||
|
|
|||
| data class LottoResult(val numbers : List<Int>, val bonus : Int) | |||
There was a problem hiding this comment.
데이터 클래스를 만들고 활용을 못하고 있습니다. 현재 파일 말고도 해당 데이터 클래스를 사용하면 일관성과 유지보수성이 높아질 것 같아요
| userTickets.add(ticket) | ||
|
|
||
| return ticket | ||
| } No newline at end of file |
There was a problem hiding this comment.
데이터 클래스와 전역 변수와 함수가 같은 파일 안에 있는 것이 SRP를 위반합니다.
또한 전역 변수를 만들면 어디서든 접근하고 변경이 가능해서 굉장히 좋지 못합니다.
캡슐화 할 클래스를 만들고 의존성을 주입하는 방식으로 변경해 보세요
| bonus = Random.nextInt(1, 46) //1~45까지 | ||
| } while (bonus in numbers) | ||
| return bonus | ||
| } No newline at end of file |
There was a problem hiding this comment.
공통되고 반복되는 상수가 많을 경우 enum 이나 정적으로 선언해서 사용하면 유지보수가 훨씬 좋아질 것 같아요
| THIRD(5,false,1500000), //150만원 | ||
| FOURTH(4,false,50000), //5만원 | ||
| FIFTH(3,false,5000) //5천원 | ||
| } |
There was a problem hiding this comment.
위 Rank 클래스의 matchCount와 hasBonus를 전혀 사용하고 있지 않습니다.
심지어 아래 함수조차 전혀 활용을 못하고 있는데, 위처럼 만든 이유가 있을까요?
| fun printCalculate(rankCountMap: Map<Rank, Int>) { | ||
| val profitRate = calculateProfit(rankCountMap) | ||
| println("\n총 수익률은 ${String.format("%.1f", profitRate)}%입니다.") | ||
| } |
There was a problem hiding this comment.
이 파일에서 함수 활용 부분은 매우 좋다고 생각합니다.
getOrDefault 도 꽤 어려울 수 있었을 텐데 잘 사용하셨고, forEachIndexed, sortedBy 등 응용을 꽤 잘하신 것 같아요
|
|
||
| if (numbers.size != 6) throw IllegalArgumentException("size") | ||
| if (numbers.toSet().size != 6) throw IllegalArgumentException("duplicate") | ||
| if (numbers.any { it !in 1..45 }) throw IllegalArgumentException("range") |
There was a problem hiding this comment.
위에서 enum이나 sealed 를 사용하라고 했던 부분입니다.
두군데에 같은 클래스의 상태를 주면,
- 두 파일을 이동할 필요없이 하나의 클래스 내부 값 수정만으로 수정이 편해지게 되고
- 오타가 날 확률도 줄어들게 되고
- 사용 위치를 파악하기 편하게 되는 등
장점이 굉장히 많아지므로 생각해 보시면 좋겠습니다.
2 participants
이번 과제에서는 최대한 **단일 책임 원칙(SRP)**을 지키는 방향으로 구조를 설계해보았습니다.
코드를 작성하기 전에 먼저 알고리즘을 구성한 뒤, 각 역할별로 클래스를 분리하여 유지보수성과 가독성을 높이고자 노력했습니다.
혹시 이 외에도 더 개선할 점이나 고려해야 할 부분이 있다면, 편하게 피드백 주시면 감사하겠습니다!
참고로 현재 브랜치는 총 3개이며,
이번 과제 구현본은 Shinseojin-StudyGroup 브랜치입니다.
(shinseojin 브랜치는 과제와는 별도로, 개인적인 규칙을 적용해 실험해본 브랜치입니다.)