통합 테스트를 포함하고 컨트롤러를 리팩토링 #7
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🐛 Fix : 버그 수정 🔧 Modify : 파일/폴더 수정/삭제/위치변경 🎨 Style : 코드 포맷 변경, 세미 콜론 누락, 의미 없는 코드 수정 💄 Design : CSS등 사용자 UI/UX 디자인 변경 🤖 Refactor : 코드 리팩토링 📝 Docs : 문서 수정 💡 Comment : 필요한 주석 추가 및 변경 ✅ Test : 테스트 코드 추가 🚚 Chore : 빌드 업무 수정, 패키지 관리자 구성 업데이트 등 환경설정 관련사항 📝 Docs : 문서 수정 레이어드 아키텍처에 따른 기능 목록 작성
구현되지 않은 기능에 대한 테스트 주석처리
역할에 따라 기능 목록 수정 1. 데이터 유효성 검사는 각 데이터 저장 클래스나 레코드가 담당 2. ErrorException은 계층에 따라 나누기 애매하기 때문에 util로 분리 3. 당첨 번호와 보너스 번호 저장을 위해 사용했던 클래스 NanumLotto에서 LottoCompany로 이름 수정
📝 Docs : 문서 수정 1. Customer class에 싱글톤 패턴을 적용해서 Application Layer의 여러 클래스가 동일한 값에 접근할 수 있도록함. 2. 피드백에 따라 docs/Readme.md를 수정 3. 예외를 처리하기 위해 ExceptionStatus를 정의
📝 Docs : 문서 수정 ✅ Test : 테스트 코드 추가 1. Lotto record에 유효성 검사 로직 추가 2. Lotto 유효성 검사 로직 테스트 추가
✅ Test : 테스트 코드 추가
1. Customer 클래스 필드에 getter 붙임
- lottos 필드에는 setter를 붙였는데 객체 생성 이후에 lottos 필드를 초기화하기 위함.
2. LottoCompany 싱글톤 생성
3. LottoCompany의 당첨 번호와 Lotto의 생성 번호의 유효성 검사 로직이 중복되어 LottoValidator가 획일적으로 관리
📝 Docs : 문서 수정 1. Winnings enum 클래스 생성 - 당첨금과 코멘트를 저장 2. Winnings는 데이터를 저장하기 보단 유용하게 사용되는 클래스기 때문에 Util 패키지로 옮김 그에 따라 Readme도 수정
🤖 Refactor : 코드 리팩토링 💡 Comment : 필요한 주석 추가 및 변경 ✅ Test : 테스트 코드 추가 1. 객체의 독립성을 위해 Customer를 singleton 패턴으로 생성하는 대신 record로 관리 2. 그에 따라 CustomTest 수정 3. Lottogenerator 생성과 createCustomer 메서드 작성 그리고 정상동작을 테스트 4. InputManager와 OutputManager를 둬서 추후 입출력 구현을 수정할 수 있도록함
1. Lotto에 toString 오버라이드 2. LottoGenerator에 createLottos 메서드 생성 역할 분리를 위해 로또 1개를 생성하는 createLotto도 private으로 생성
1. 로또 게임 동작 완료 2. LottoGenerator 와 LottoResultCalculator 테스트 수행 필요 3. Winnings enum에 valueOfWinningsAndBonus를 추가해서 당첨 번호와 보너스 번호로 등수 찾기 기능 제공
✅ Test : 테스트 코드 추가 LottoCompany를 생성할 때 로또 개수를 6개가 아니게 입력하면 예외가 발생하도록 수정 및 테스트 작성
🤖 Refactor : 코드 리팩토링 1. LottoManager에 필드에 객체 생성하는 것 보다는 로컬 변수로 저장 2. 로직으로 이미 충분히 Lotto의 제약사항을 만족할 수 있기 때문에 try - catch 문 제거 3. InputManager와 OutputManager를 필드로 둬서 LottoResultCalculator와 LottoGenerator 클래스 안에 메서드가 공통적으로 입력받는 인자 제거 4. Winnings enum에서 newWinningsMap에서 생성하는 Map은 입력 순서를 보장하는 LinkedHashMap을 사용 5. SystemOutputManager는 요구사항을 맞추기 위해 "당첨 통계"를 출력하도록 함
1. Winnings와 LottoResultCalculator.checkWinningElement 메서드를 읽기 쉽게 수정
1. LottoGenerator와 LottoResultCalculator는 데이터를 이용한 비즈니스 로직을 처리하기 때문에 model/service로 변경 2. 사용자의 입력을 검사하고 비즈니스 로직을 통제하는 LottoController에서 사용자 입력 유효성 검사 수행 3. 당첨 번호와 보너스 번호를 저장하는 클래스의 이름을 LottoCompany에서 PrizeNumbers로 변경 4. 모든 유효성 검사 로직을 LottoVaildator에 위치시키고 해당하는 테스트 작성
1. LottoGenerator와 LotoResultCalculator의 메서드 인자를 더 간소화시켜서 테스트 용이성 증대 2. LottoGenerator와 LotoResultCalculator의 퍼블릭 메서드에 대해 테스트 작성
1. Winnings 리팩토링 시작
1. Money DTO 생성 2. Lotto에게 로또 구매를 위한 Money 유효성 검사 역할 부여
1. 당첨 번호와 보너스 번호의 유효성 검사를 PrizeNumbers에게 역할을 재배치
✅ Test : 테스트 코드 추가 1. 수정된 Winnings에 맞게 LottoResultCalculator를 수정 2. Lotto에 유효성 검사 역할을 부여하고 그에 맞는 테스트 코드 작성 3. 출력을 작성할 때 당첨 통계를 보여줄 수 있도록 수정
1. 사용자 입력에 대한 테스트 작성
✅ Test : 테스트 코드 추가 1. LottoGenerator 내부에서 Random을 생성하기 때문에 테스트하기 어려움 따라서 외부에서 Random 객체를 주입하는 방식 사용 2. Mockito를 이용해서 Random 클래스의 모킹 객체 생성 3. 사용자 입출력에 대한 대체 방법 제시 4. 통합 테스트를 위한 모든준비가 끝났다.
1. Controller를 위한 통합 테스트 작성 2. Input 받는 방식을 테스트가 용이하기 위해 Scanner를 주입 받아 사용하도록 수정
✅ Test : 테스트 코드 추가 1. 컨트롤러에서 람다식을 이용해 사용자 입력 받는 로직과 유효성 검사하는 로직을 분리. 추후에 유효성을 검사하는 로직이 바뀌면 다른 로직을 주입하면 되기 때문에 더 유연함. 2. 통합 테스트를 통해 리팩토링한 컨트롤러의 동작을 검사
1. Lotto와 관련된 예외를 모아서 model 패키지로 이동 2. Money에 존재하는 예외 메시지는 Lotto와 관련있지 않고 Money Dto 그 자체적인 정의기 때문에 따로 빼두지 않음
ooMia
reviewed
Mar 13, 2024
There was a problem hiding this comment.
MAXIMUM_NUMBER처럼 대문자와 언더바를 활용해서 명명하길 추천하고 있네요
내부적으로 합의한 바가 없기 떄문에 참고만 하면 될 것 같습니다.
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| // 생성되고 더 이상 수정되지 않음 |
There was a problem hiding this comment.
생성되고 더 이상 수정되지 않음
어떤 의도로 작성한 주석인가요?
There was a problem hiding this comment.
Java 19에 대한 의존성을 만든 계기와 이유가 무엇인가요?
요구사항에서는 JDK 17에서의 테스트 성공을 명시해두었는데, 저희가 완전히 동일한 요구사항을 바탕으로 진행하고 있는 것이 아니므로 근거가 필요할 것 같습니다.
There was a problem hiding this comment.
프로그램 테스트나 실행하면서 자동으로 만들어졌나보네요 지우겠습니다!
There was a problem hiding this comment.
접두사 [ERROR]에 대한 규칙성을 유틸 클래스로 분리할 수도 있었을텐데 판단 근거가 무엇인가요?
There was a problem hiding this comment.
참고) int 또는 long 형의 최댓값에 대한 테스트도 해볼 수 있습니다.
| Random randomNumberMock = Mockito.mock(Random.class); | ||
| when(randomNumberMock.nextInt(45)) | ||
| .thenReturn(1,33,32,31,30,34) | ||
| .thenReturn(1,31,9,17,23,23,23,44); // 23이 중복으로 생성돼도 무시하고 다음 값을 받는다. |
There was a problem hiding this comment.
저는 이 부분을 오히려 무시하지 않고 예외로 처리했어요
솔은 어떤 판단으로 이렇게 작성했나요?
| } | ||
|
|
||
| private HashMap<Winnings, Integer> newWinningsMap(){ | ||
| HashMap<Winnings, Integer> map = new LinkedHashMap<>(); // 입력 순서를 보장하면서 Map을 사용할 수 있는 자료형 |
There was a problem hiding this comment.
저는 TreeMap을 사용했는데,
메모리-성능 트레이드 오프를 고려했을 떄,
Lotto에서는 작은 크기의 맵에 대해 다수의 반복 탐색을 시도하기 떄문에
LinkedHashMap을 사용하는게 더 적절한 것 같아요
| } | ||
| private void validateMoney(int money){ | ||
| if(money < 0 ) | ||
| // Lotto와는 관련 없는 에러 메시지기 때문에 따로 enum 처리하지 않음 |
There was a problem hiding this comment.
우테코에서는 모든 상수를 Enum 처리하라고 가이드 하더라구요
별도의 파일로 분리하기 애매해서 그런거라면, 내부 Enum으로 처리해도 괜찮을 것 같아요
There was a problem hiding this comment.
참고) SoC를 신경써서 확장 가능한 프로젝트 구조를 고안하고 있어요
통합 구조 제안
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
Type of Change
(Delete options that are not relevant)New feature(non-breaking change which adds functionality)Breaking change(fix or feature that would cause existing functionality to not work as expected)How Has This Been Tested?
Features / Changes / TO-DO: