[로또] 남궁혜민 미션 제출합니다. #2

hyeminililo · 2024-05-13T10:30:21Z

궁금한 점

1.이번 과제에서는 test 코드를 먼저 짜고, 그에 맞게 로직을 구현하는 TDD를 이용해 구현해 봤는데, 처음 TDD를 써보니, 많은 어려움을 겪었습니다. test 코드에서 비즈니스 로직에서 필요한 메서드들을 짜려고 했는데, 연결이 되지 않은 상태에서 test 코드를 먼저 짜서 어려웠습니다. test 코드를 짜고, test 코드를 이용해서 컨트롤러 로직을 짜고 싶은데, 접근을 어떻게 해야하는지 궁금합니다.
2. test 코드에 실제로 tickets와 같은 로직에 사용되는 클래스들을 이용해서 테스트 코드를 짜는게 더 좋은 방법인지 , 아니면 test 코드를 짤 떄는 tickets와 같은 모델들은 일단 강제적으로 지정을 하고, 구현을 하는게 더 좋은 방법인지 궁금합니다.
3. tickets와 당첨 번호를 관리하는 Winning이라는 클래스를 만들었는데, Rank라는 클래스와 역할이 비슷한 것 같아, 역할을 Rank로 위임을 했습니다. 조금 더 세분화 해서 코드를 짜고 싶은데 , 어떻게 로직을 나눠야하는지 궁금합니다.

1차 리팩토링 후

리뷰를 중심으로 리팩토링을 진행했습니다 :)

Tickets에 대한 역할 분배 리팩토링
0이나 하한에 대한 Test 코드 작성
객체를 생성하는 로직에 대한 리팩토링

당첨 결과를 확인하는 테스트 코드 구현

불필요한 코드 삭제

WinningTest 코드 삭제

tickets 클래스에 대해 역할을 세분화

ticket 클래스를 관리하는 ticket 컨트롤러 클래스 작성

티켓 클래스의 역할 분리 관련해 구조 변경

vact19

혜민님 고생하셨습니다!

각 회차의 미션을 진행할 때마다 제출하신 코드의 가독성과 안전성이 올라가는 느낌을 받는데, 이번엔 특히 더 그런 느낌이 드네요!

그래서 이건 안 좋은 것 같다보다는 이건 정말 좋았다혹은 이건 이렇게 바꿔보면 더 좋겠다 싶은 느낌이 훨씬 더 많았는데, 이번 리팩토링에서는 코드의 변경 도 물론 좋지만 여러 대안을 세우고 각 대안을 비교하며 고민에 집중하는 것도 유익할 것 같아요!

제가 리뷰가 늦어 이미 리팩토링을 진행 중이신 것 같아요. 아직 작성중이신 부분이 있을 수도 있지만 일단은 1차 리팩토링이 적용된 최신본을 기준으로 리뷰했습니다!

궁금한 점

1.이번 과제에서는 test 코드를 먼저 짜고, 그에 맞게
로직을 구현하는 TDD를 이용해 구현해 봤는데, 처음 TDD를 써보니, 많은 어려움을 겪었습니다. test 코드에서 비즈니스 로직에서 필요한 메서드들을 짜려고 했는데, 연결이 되지 않은 상태에서 test 코드를 먼저 짜서 어려웠습니다. test 코드를 짜고, test 코드를 이용해서 컨트롤러 로직을 짜고 싶은데, 접근을 어떻게 해야하는지 궁금합니다.

이 문제에 대해서는 저는 잘 모르겠습니다. TDD를 깊게 공부해본 적이 없거든요!
하지만 다른 개발자분들의 말을 인용해 보자면
테스트 코드 작성 자체가 익숙하지 않다면, 일단은 어떤 방법론을 쫓기보다는 테스트 코드 작성 자체에 대한 지식과 경험을 쌓는 게 좋다
라는 말을 따라도 될 것 같습니다.

테스트 방법론보다는 올바른 테스트가 더 중요하니까요!
이번 미션은 TDD의 '정복'보다는 '체험'에 의의를 둬도 좋을 것 같아요.

test 코드에 실제로 tickets와 같은 로직에 사용되는 클래스들을 이용해서 테스트 코드를 짜는게 더 좋은 방법인지 , 아니면 test 코드를 짤 떄는 tickets와 같은 모델들은 일단 강제적으로 지정을 하고, 구현을 하는게 더 좋은 방법인지 궁금합니다.

질문을 잘 이해 못했어요! 제가 소스코드를 한번 읽어 보았으니, 정확히 무엇을 구현하는 상황에서 고민을 하게 되었는지 자세히 설명해주시면 이해 갈 것 같아요!
그리고 '구현'이란 프로덕션 코드의 구현인가요 테스트 코드의 구현인가요?

tickets와 당첨 번호를 관리하는 Winning이라는 클래스를 만들었는데, Rank라는 클래스와 역할이 비슷한 것 같아, 역할을 Rank로 위임을 했습니다. 조금 더 세분화 해서 코드를 짜고 싶은데 , 어떻게 로직을 나눠야하는지 궁금합니다.

비슷한 역할을 하는 클래스가 둘이 있고, 분리해야 할지 확신이 들지 않는다면
일단 합친 다음에 나중에 분리를 고려해도 될 것 같아요.
소스코드 파일의 수가 늘어나는 것은 관리 포인트 증가의 문제가 되기도 하니까요!

이런 애매한 상황은 명확한 답이 없지만,
제가 느껴온 것들을 기반으로 어떤 생각의 틀?을 공유하자면

1. 데이터베이스 등과 연관되어 있어서 한 번 설정하면 바꾸기 힘든 것이 아니라, 단순히 역할 수행 책임과 같은 경우에는 
`(나름의 근거가 있다면)일단 빠르게 구현`한 다음, 이후 생각해본다. 
좋은 코드를 만들어내는 것 만큼 '제 시간에' 코드를 만들어내는 것도 중요하기 때문.

2. 좋은 코드나 좋은 설계에 관한 생각들의 상당 부분은 `변경`이라는 키워드에 집중한다. (예를 들면: 주석을 남용하지 마라! 소스코드의 변경을 주석이 따라가지 못할 경우 만들어지는 잘못된 주석은 혼란을 가중시키기 때문이다)
이 코드가 나중에 '변경'되었을 때도 이 구조가 유지될 수 있을지 생각해보면 좋다.
여기서 '변경'이란 소스코드의 추가, 삭제, 구현 수정 등을 모두 포함한다.

이러한 생각의 틀을 가지고 고민해보면 도움이 될 수 있을 것 같아요!

vact19 · 2024-05-17T14:06:26Z

src/main/kotlin/lotto/controller/Controller.kt

+    // 시작하는 컨트롤러
+    fun start() {
+        val purchaseAmount = inputPurchaseAmount()
+        generateLottoTickets(purchaseAmount)


제가 처음 코드를 읽을 때는
generateLottoTickets의 반환값이 없는 것에 의아함을 느끼고
메서드 내부를 들여다본 후, 결국 TicketController의 내부 구현까지 보고 나서야
해당 클래스에서 값을 생성하고 저장한다는 것을 알 수 있었습니다.
반환값을 받은 다음 이후 로직에서 값을 넘긴다든지, 생성과 저장 로직을 분리한다든지, 반환값을 사용하지 않으려면 메서드 이름을 [save-store]등으로 바꾼다든지... 하는 식으로 바꿔도 좋을 것 같아요.

사실 위에 강조표시한 방법 하나하나가 중요하다기보다는,
정말 중요한 것은
start 메서드를 위에서 아래로 쭉 읽었을 때 빠르게 이해되게끔 만드는 것 같아요!

vact19 · 2024-05-17T14:08:11Z

src/main/kotlin/lotto/controller/TicketController.kt

+    }
+
+    // tickets에 ticket을 추가하는 메소드
+    fun addTicket(ticket: List<Int>) {


요건 private으로 하는게 좋을 거 같네요. 접근 제어자는 private부터 시작해서 점점 넓혀 봅시다!
아니면 tickets.add를 바로 호출해도 될 것 같아요!

vact19 · 2024-05-17T14:10:21Z

src/main/kotlin/lotto/model/Bonus.kt

+import lotto.util.Validator.validateLottoBonusDuplicate
+
+class Bonus (
+    private val lotto: Lotto,


전체적으로 VO 방식을 전체적으로 잘 활용하신 것 같아요.
값을 클래스로 감싸고 생성자 레벨에서 검증하여 코드의 신뢰성을 보장하고, 규칙을 한 곳에서 관리하면
당장 관리해야 할 소스코드 파일의 수는 늘어나지만 여러 이점이 있죠.

요구사항에서 'LOTTO 클래스를 그대로 사용하라'라고 했는데
이 요구사항의 의도를 잘 파악하신 걸로 생각됩니다.

물론 모든 타입을 클래스로 감쌀 필욘 없지만, Lotto, Bonus는 게임 내에서 중요한 역할을 하는 요소이고
이러한 요소들에 대한 규칙은 한 곳에서 정의하여 관리하는 것이 좋기 때문에 좋은 결정 같습니다.

vact19 · 2024-05-17T14:12:25Z

src/main/kotlin/lotto/util/ErrorMessage.kt

+    LOTTO_DUPLICATE("당첨 번호는 각기 다른 수를 입력해야 합니다."),
+    BONUS_DUPLICATE("보너스 번호는 당첨 번호와 중복되어서는 안됩니다.");
+
+    fun getMessage(): String = "[ERROR] $message"


이런 거 좋네요. 무작정 구현하다가 [ERROR]까지 밖에서 하드코딩으로 입력하게 하는 실수를 할 수도 있는데,
그러면 관리가 번거롭고 오타 낼 확률이 높죠.

이렇게 메서드 내부에서 [ERROR] String을 선언해서 이 메서드의 사용자가 특정한 형식보단 에러 메시지에 집중할 수 있게 됐네요!

vact19 · 2024-05-17T14:12:51Z

src/main/kotlin/lotto/util/Validator.kt

@@ -0,0 +1,64 @@
+package lotto.util
+
+object Validator {


지금 흐름을 읽는 것이 크게 불편하진 않지만, 하나의 Validator에서 모든 것을 하는 게 아니라 여러 Validator로 나눠보는 시도도 괜찮을 것 같습니다.

예를 들어 로또 게임 룰에 관한 Validator와 입력값에 관한 Validator로 나눈다든지
하는 일에 따라 [로또번호-상금-순위] Validator로 나눈다든지 그런 거요.

그런데 지금은 입력값 형식과 로또 번호에 관한 검증밖에 없기 때문에
분리가 꼭 필요하다고 생각이 들진 않고, 분리의 효과가 크게 느껴지진 않을 것 같아요.
그렇지만 생각만 한번 하고 넘어가도 좋은 경험이 될 것 같네요.

vact19 · 2024-05-17T14:13:29Z

src/test/kotlin/lotto/BonusTest.kt

+    @DisplayName("보너스 번호는 1부터 45 사이여야 한다.")
+    fun should_ThrowException_When_OutOfRange() {
+        assertThrows<IllegalArgumentException> {
+            Bonus(lotto, 46)


0처럼 하한 경계값도 테스트해주시면 더 좋을 것 같습니다. 이럴 땐 ParameterizedTest도 괜찮아 보여요!

vact19 · 2024-05-17T14:17:59Z

src/test/kotlin/lotto/PrizeTest.kt

+    private val lotto = Lotto(listOf(1,2,3,4,5,6))
+    private val bonus = Bonus(lotto, 7)
+
+    private val tickets = listOf(listOf(1,2,3,7,8,9), listOf(1,2,3,4,10,11), listOf(11,12,13,14,15,16), listOf(17,18,19,20,21,22), listOf(23,24,25,26,27,28))
+
+    private val rank = Rank(lotto, bonus)
+    private val prize = Prize(rank)


지금은 문제가 되는 상황이 아니지만, 이렇게 필드에서 값을 관리할 경우 가변성 등의 문제 때문에
여러 테스트가 값을 공유하여 가변성 등의 문제로 매번 결과가 달라질 수도 있으니 주의!

공유해서 쓸 만한 이유가 있는 불변 값이면 클래스 멤버로 선언해도 부작용은 없지만,
테스트는 독립성이 중요하기에 가능한 한 값의 공유를 피하고 지역변수(스택 영역) 내에서 해결하는 습관을 들이는 것이 좋아 보여요!

테스트 대상 객체를 지역변수 영역에서 생성하고 싶은데,
객체를 생성하는 로직이 너무 복잡하고 반복된다! 싶을 땐
해당 로직을 private method로 묶어서 호출해도 됩니다!

hyeminililo · 2024-05-18T10:55:29Z

궁금한 점 2번은 리팩토링 과정에서 답을 찾았습니다 ! 제가 test 코드를 짜야하는 이유를 잘못 알고있어서 생긴 것 같습니다. 실제 프로덕션 코드에 있는 객체를 test 코드에 사용하면 안된다고 오해를 하고 있어서 2번과 같이 질문을 했던 것 같습니다 ,,

Controller 클래스를 주로 리팩토링 진행

중복되어 사용하고 있던 RankingTest 삭제

0이나 하한 값을 테스트 하는 코드 구현

vact19

Controller쪽을 중심으로 코드가 변경되었는데,
전체적으로 변경 전보다 흐름 파악이 더 어려워진 느낌을 받았습니다.

메서드의 이름은 작업사항을 명확히 설명해야 한다.
클래스 내의 메서드들에서 public과 private을 명확히 구분하여 역할을 배분한다 (모두 public으로 만들지 않는다)
public method만을 읽고도 흐름 파악이 되어야 한다. (읽는 사람이)흐름이 아닌 상세 구현이 알고 싶을 때 private method를 찾아가게 만들어야 한다.

위 3가지 사항을 중심으로 리팩토링을 하면 좋을 것 같습니다!

또한 Controller가 맡고 있는 역할이 너무 많은 것 같습니다. Controller 내부에서 많은 것을 하려고 하기보다 다른 주요 객체의 로직을 '호출'만 하는 그림으로 만드는 것이 이해하기 쉬울 것 같습니다!

vact19 · 2024-05-20T08:59:31Z

src/main/kotlin/lotto/controller/Controller.kt

+        purchaseController()
+        inputWinningNumberCountroller()


메서드의 이름은 그 메서드가 하는 일을 설명해야 하는데,
이 작업의 이름을 purchaseController 혹은 inputWinningNumberController로 지어도 될까요?

vact19 · 2024-05-20T09:00:30Z

src/main/kotlin/lotto/controller/Controller.kt

+        inputWinningNumberCountroller()
+    }
+
+    fun purchaseController() : List<List<Int>> {


purchaseController는 start()메서드에서도 호출되고, inputWinningNumberController()에서도 호출됩니다.
start()메서드 첫째 줄에서 수행될 필요가 있을까요?

vact19 · 2024-05-20T09:03:01Z

src/test/kotlin/lotto/ParameterizedTest.kt

+import org.junit.jupiter.api.Test
+
+
+class ParameterizedTest {


JUnit의 @ParameterizedTest를 말한 것이었습니다!
여러 가지 값을 사용해서 동작을 테스트하고 싶을 때 좋습니다.

vact19 · 2024-05-20T09:04:48Z

src/test/kotlin/lotto/ParameterizedTest.kt

+    @Test
+    @DisplayName("입력 값은 0이면 안 된다.")
+    fun should_ThrowException_When_InputValue_Is_Zero() {
+        assertFalse(inputInteger == 0)


단순히 10 == 0을 테스트하는 것은 프로그램의 품질을 보증해줄 수 없습니다.
이 프로젝트의 모든 프로덕션 코드를 지워버리더라도 반드시 테스트가 성공하기 때문이죠.
간단히 말해서, 내가 만든 클래스의 'public method'를 테스트한다고 생각하시면 됩니다!

vact19 · 2024-05-20T09:08:58Z

src/test/kotlin/lotto/PrizeTest.kt

+    @BeforeEach
+    fun setUp() {
+        lotto = Lotto(listOf(1,2,3,4,5,6))
+        bonus = Bonus(lotto, 7)
+        rank =  Rank(lotto, bonus)
+        prize = Prize(rank)
+
+    }


beforeEach로 의도치 않은 값의 공유를 막을 순 있지만, 필드에 있는 값을 사용하므로
테스트 케이스 하나를 보았을 때
어떤 조건에서 어떤 동작을 수행했을 때 어떤 결과가 나오는지를 알기가 힘든 느낌을 받았습니다.
given-when-then 패턴에 대해서 알아보고, 하나의 테스트 코드에서 그 흐름이 최대한 잘 보이도록 고민해보면 좋겠습니다!

vact19 · 2024-05-20T09:17:24Z

src/main/kotlin/lotto/controller/Controller.kt

+
+class Controller {
+    private val ticketController = TicketController()
+    private val purchaseAmount = inputPurchaseAmount()


inputPurchaseAmount()로 초기화를 하게 되면, Controller의 생성자를 호출하는 즉시 UI를 통해 사용자의 입력을 받게 됩니다. Controller의 객체를 생성할 경우 UI 로직 수행은 Controller 생성자의 역할로 기대되는 로직이 아닌 것 같습니다!

labyrinth30 and others added 30 commits May 7, 2024 23:26

Model, Test 추가

0e74b99

Purchase 클래스 추가

9d0bf41

test: Purchase 모델 테스트 코드 작성

eff3f62

feat: Purchase 모델 구현

f5c67b8

feat: 출력 뷰 구현

4e6dfcb

test: Tickets 모델 테스트

92c1bad

feat: Tickets 모델 구현

cb6823e

refactor: Bonus 모델 수정

01e5bb0

test: Bonus 모델 테스트 수정

d99ee5e

refactor: Lotto 모델 수정

72df0ce

✅ Feat : Winning 테스트 구현

62f533e

🎨 Winning 모델 구현

63ecd59

feat : Ranking 테스트 구현

7081bb4

당첨 결과를 확인하는 테스트 코드 구현

refactor: 중복 검사 로직 수정

71e9a19

test: 테스트 코드 수정

324ffb9

test: Rank 모델 테스트

2c3b332

feat: Rank 모델 구현

2583eb9

Merge branch 'main' of https://github.com/hyeminililo/kotlin-lotto

e130820

test: 상금 및 수익률 로직 테스트

7f2d4bf

feat: 상금 및 수익률 모델 구현

934dc9f

Merge branch 'main' of https://github.com/hyeminililo/kotlin-lotto

f52766c

feat : Winning 클래스 삭제

68c8f72

불필요한 코드 삭제

프로그램 구현

a368a9a

Merge branch 'main' of https://github.com/cucumber99/kotlin-lotto

7d4d9a6

feat : 불필요한 테스트 코드 삭제

0b61d78

WinningTest 코드 삭제

Merge branch 'main' of https://github.com/cucumber99/kotlin-lotto

4ee0cda

♻️ 1차 리팩토링

d5deecc

tickets 클래스에 대해 역할을 세분화

📝 Ticket 클래스 작성

4232930

📝 TicketController 작성

83ce4a4

ticket 클래스를 관리하는 ticket 컨트롤러 클래스 작성

🎨 구조 변경

2cd0a9e

티켓 클래스의 역할 분리 관련해 구조 변경

🔥 Docs : Tickets 클래스 삭제

ca64be2

vact19 self-requested a review May 17, 2024 12:09

vact19 requested changes May 17, 2024

View reviewed changes

hyeminililo added 4 commits May 19, 2024 00:53

♻️ Refactor : 2차 리팩토링

17ae544

Controller 클래스를 주로 리팩토링 진행

🔥 Feat : 불필요한 테스트 코드 삭제

2fcb14b

중복되어 사용하고 있던 RankingTest 삭제

📝 Test : ParameterizedTest 테스트 코드 구현

3c62fb7

0이나 하한 값을 테스트 하는 코드 구현

✅ Test : 2차 리팩토링에 대한 테스트 성공

49fe198

hyeminililo requested a review from vact19 May 19, 2024 19:03

vact19 requested changes May 20, 2024

View reviewed changes

✅ Refactor : 3차 리팩토링

e4d7fc3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[로또] 남궁혜민 미션 제출합니다. #2

[로또] 남궁혜민 미션 제출합니다. #2

hyeminililo commented May 13, 2024 •

edited

Loading

vact19 left a comment

vact19 May 17, 2024

vact19 May 17, 2024

vact19 May 17, 2024

vact19 May 17, 2024

vact19 May 17, 2024

vact19 May 17, 2024

vact19 May 17, 2024

hyeminililo commented May 18, 2024

vact19 left a comment

vact19 May 20, 2024

vact19 May 20, 2024

vact19 May 20, 2024

vact19 May 20, 2024

vact19 May 20, 2024

vact19 May 20, 2024

		import org.junit.jupiter.api.Test


		class ParameterizedTest {

[로또] 남궁혜민 미션 제출합니다. #2

Are you sure you want to change the base?

[로또] 남궁혜민 미션 제출합니다. #2

Conversation

hyeminililo commented May 13, 2024 • edited Loading

궁금한 점

1차 리팩토링 후

vact19 left a comment

Choose a reason for hiding this comment

궁금한 점

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyeminililo commented May 18, 2024

vact19 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyeminililo commented May 13, 2024 •

edited

Loading