[CT-2-2-3] 차트게임 기능 - 실행 테스트 및 버그 수정 #35

nosorae · 2024-06-06T09:03:42Z

PR 순서: [CT-2] <- [CT-2-2-2] <- [CT-2-2-3] <- [CT-5] <- [CT-5-1] <- [CT-37] <- [CT-5-2] <- [CT-5-3-1] <- [CT-5-3-2] <- [CT-5-3-3]

Overview

[CT-2-2-2] 차트게임 기능 - presentation 레이어 - Screen 에 컴포넌트 배치하고 ViewModel과 연결 #34 를 먼저 보시는 게 순서에 맞습니다.
차트게임 기능 초기 코드를 [CT-2-2-2] 에서 완성하고나서 발견된 버그들을 수정했습니다.
커밋메시지에 fix: 태그가 붙은 커밋에서 수정된 버그들을 확인하실 수 있습니다.
디자인 수정은 따로 이슈([CT-24] UI 다듬기 - 1h #24)로 만들어 놓긴 했지만, 차마 눈뜨고 보기 힘든 부분들을 추가로 수정하여 design: 태그를 붙여 커밋했습니다.

깨닳은 점

코드를 작성하고 PR을 올리기 전에 테스트 코드를 작성해야겠다는 점입니다.
테스트 코드를 작성하지 않으면 domain 만 있을 때는 실행해서 테스트하기 어렵고 어떻게든 테스트한다 한들 테스트를 쓰기위해서 작성한 코드들을 1회성으로 버려야하니 가성비도 안나오고 실수로 지우지 않으면 리뷰어가 리뷰하기 힘듭니다.
테스트 코드를 작성했다면 현재 PR의 fix: 태그가 붙은 거의 모든 커밋이 사라졌을 것입니다.
테스트 코드를 작성하면 작성한 실행코드를 수정하지 않고 지속 가능한 테스트가 가능하겠다는 생각이 들었습니다.

…eature/ct-2-2-3-chart-game-bug-fix

nosorae · 2024-06-06T15:38:53Z

data/src/main/java/com/yessorae/data/repository/ChartRepositoryImpl.kt

        }
+
+    private suspend fun fetchNewChartRandomlyWithRetry(


설마 S&P500에 1년미만인 종목이 있을까 하며 처리 안하고 넘어갔던 부분인데, TotalTurn 보다 틱수가 낮으면 게임이 불가하여 TotalTurn 보다는 큰 게 나올 때까지 시도하되 무한 루프를 돌면 안되니 최대 횟수를 두었습니다.

domain/src/main/java/com/yessorae/domain/common/Result.kt

…ofit, rateOfProfit 값이 게임 중 누적되게 수정), 주석 추가, 진행률 0~1범위로 수정

f-lab-nathan · 2024-06-09T12:04:13Z

data/src/main/java/com/yessorae/data/repository/ChartRepositoryImpl.kt

        }
+
+    private suspend fun fetchNewChartRandomlyWithRetry(


Q; 이 역할을 가진 함수를 분리한 이유가 있나요?

fetchNewChartRandomly 함수를 사용하는 입장에서 알 필요 없는 currentRetryCount 같은 파라미터를 노출하지 않고 withContext 로 디스패칭을 바꿔주는 것을 한 번만 할 수 있어서 분리했습니다.

f-lab-nathan · 2024-06-09T12:07:11Z

data/src/main/java/com/yessorae/data/repository/ChartRepositoryImpl.kt

+            )
+
+        // 서버에서 가져온 차트가 totalTurn 보다 작으면 다시 요청
+        if ((dto.ticks?.size ?: 0) < totalTurn) {


nit; 옵셔널을 바로 해제해서 푸는 것보다 위에서 ticks가 널일 때 핸들링 하는 코드를 넣는게 어떨까요?
nit2; dto의 ticks를 널 대신 빈 리스트로 초기화 하는 것은 어떨까요?

넵 두 번째 방법이 좋다고 생각합니다. 불필요한 null 핸들링이라고 생각합니다. 감사합니다!

f-lab-nathan · 2024-06-09T12:08:09Z

domain/src/main/java/com/yessorae/domain/entity/ChartGame.kt

-
-    val totalCommission: Money = Money(trades.sumOf { trade -> trade.commission.value })
-
+    // 현재 턴의까지의 차트 데이터
    val visibleTicks: List<Tick> =
        chart.ticks
            .sortedBy { it.startTimestamp }
            .subList(0, chart.ticks.size - totalTurn + currentTurn - 1)


Q; subList의 인덱스가 항상 범위 내에서 동작할까요?

결론부터 말씀드리자면, 여기서도 범위 핸들링을하는 게 좋겠습니다. (보여줄 수 있는 만큼만 보여주기)

물론 현재 브랜치부터는 Tick 개수가 totalTurn 보다 작으면 에러를 내리게 해서 동작합니다.
따라서 현재 코드상으로는 코멘트 주신 코드로 에러가 발생할 일은 없습니다.

하지만 생각해보니 애초에 N번 재시도 하고 그래도 Tick 개수가 totalTurn 보다 적으면 에러 내리는 방식이 좋은 UX는 아닌 것 같네요

그래서 해결방법을 두 가지 생각해봤는데요

운영으로 푼다. 상장한지 3년 이상인 종목만 랜덤풀에 두고 차트 데이터를 가져온다.

부족한 대로 그 만큼만 보여준다. (예를들어 유저가 설정한 totalTurn이 100인데 봉이 100개가 아닌 78개라면 78개만 보여준다.)

결론은 두 가지 다 적용하자 입니다.

운영으로 풀고 혹여나 실수가 일어나도 무작정 에러를 내려주는 일은 막자는 생각입니다.

운영적인 것은 천천히 적용하고 2번을 먼저 적용했습니다.

f-lab-nathan · 2024-06-09T12:08:50Z

domain/src/main/java/com/yessorae/domain/entity/ChartGame.kt

+        if (trade.type.isBuy()) {
+            trade.totalTradeMoney.value
+        } else {
+            -(trade.totalTradeMoney.value)


nit; 불필요한 소괄호인 것 같습니다.

앗 제거 하겠습니다. 감사합니다.

f-lab-nathan · 2024-06-09T12:09:41Z

presentation/src/main/java/com/yessorae/presentation/ui/chartgame/ChartGameScreen.kt

+        screenEvent.collectLatest { event ->
+            when (event) {
+                is ChartGameEvent.InputBuyingStockCount -> {
+                    context.showToast(context.getString(R.string.chart_game_toast_trade_buy))


nit; 코드의 간결함을 위해 반복되는 코드는 확장함수등으로 만들어 보는 것은 어떨까요?

넵 그게 좋겠습니다. 감사합니다!

f-lab-nathan · 2024-06-09T12:10:41Z

presentation/src/main/java/com/yessorae/presentation/ui/chartgame/ChartGameViewModel.kt

-                }
-            )
+    private fun updateGameData(data: ChartGame) =
+        with(data) {


nit; data의 복사함수를 따로 정의해서 재사용하는 것도 좋을 것 같습니다.

넵 동의합니다. 만들어보겠습니다,

nosorae · 2024-06-10T14:54:52Z

👀 이모지를 붙은 4개의 리뷰를 반영했습니다.

sonarqubecloud · 2024-06-10T14:55:12Z

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

nosorae added 24 commits June 6, 2024 16:52

[CT-2-2-3] design: 상태바 아이콘 컬러 반전

85cada9

[CT-2-2-3] design: 주문 UI 얼라인 바텀 처리

3e0e04c

[CT-2-2-3] design: 매매 주문 Ui ShortCurChip 패딩/간격 수정

0573512

[CT-2-2-3] fix: NumberFormatException 으로 발생하는 크래시 수정

012f3cd

[CT-2-2-3] design: TradeOrderUi 키패드 애니메이션 제거

c9f5535

[CT-2-2-3] design: TradeOrderUi 모달바텀시트로 변경 (뒷배경, 클릭 방지, 애니메이션 등의 이유)

c0c9c88

[CT-2-2-3] style: 불필요 필드 제거

a922675

[CT-2-2-3] fix: TradeOrderUi KeyPad 버그 수정, userInput non-null로 변경

568e9ac

[CT-2-2-3] design: TradeOrderUi 금액 소수점 두 자리수로 자르기 (함수 하나로 통일하게 리팩토링 예정)

d3cadd8

[CT-2-2-3] design: TradeOrderUi 하단 버튼 shape 수정

d2f060d

[CT-2-2-3] design: 로딩 프로그래스 바 추가

17809f7

[CT-2-2-3] fix: currentGameProgress 를 % 정수가 아니라 소수로 표현되게 수정

068198d

[CT-2-2-3] fix: TopAppBar 초기 상태는 타이틀로 보여주기

d3ccf97

[CT-2-2-3] design: ChartGameBottomBarUi 에 디바이더 추가, 간격조정

afd02be

[CT-2-2-3] fix: ChartGameBottomBarUi 매도매수 버튼 활성화 조건 수정

c949dc7

[CT-2-2-3] fix: 다음 턴(틱) 보여주는 기능 정상화

c123aab

[CT-2-2-3] fix: NaN 처리 추가

af50e7c

[CT-2-2-3] fix: Flow<Result<Unit>> 일 때 Success 값 오지 않는 이슈 수정

c6d1c4d

[CT-2-2-3] fix: 매매기능 정상화, 매도 주문 커스텀 소프트 인풋에 대해 toInt 누락 추가 처리

8568076

[CT-2-2-3] fix: 불러온 차트가 틱이 50개 미만이면 게임을 정상적으로 진행할 수 없으니 재시도와 가이드 추가

95391ad

[CT-2-2-3] style: ktlintFormat

2c69b2a

[CT-2-2-3] fix: 게임 종료 정상화

5fa51fd

[CT-2-2-3] fix: 버튼 활성화 여부 조건에 게임종료인 경우 고려

4310b54

Merge branch 'feature/ct-2-2-2-chart-game-presentation-screen' into f…

c47db0b

…eature/ct-2-2-3-chart-game-bug-fix

nosorae added the feature label Jun 6, 2024

nosorae added this to the 최소 기능 구현 milestone Jun 6, 2024

nosorae requested a review from f-lab-nathan June 6, 2024 09:03

nosorae self-assigned this Jun 6, 2024

nosorae mentioned this pull request Jun 6, 2024

[CT-2-2-2] 차트게임 기능 - presentation 레이어 - Screen 에 컴포넌트 배치하고 ViewModel과 연결 #34

Merged

[CT-2-2-3] refactor: Screen 으로부터 이벤트 분기를 분리

9019b5a

nosorae commented Jun 6, 2024

View reviewed changes

domain/src/main/java/com/yessorae/domain/common/Result.kt Show resolved Hide resolved

nosorae marked this pull request as draft June 8, 2024 17:53

nosorae added 2 commits June 9, 2024 10:34

[2-2-3] fix: 전량 매도시에 totalProfit, rateOfProfit 이 0이 되는 버그 수정 (totalPr…

7ee037b

…ofit, rateOfProfit 값이 게임 중 누적되게 수정), 주석 추가, 진행률 0~1범위로 수정

[CT-2-2-3] fix: 음수일 때도 버튼 활성화되게 수정

59d2c0f

nosorae marked this pull request as ready for review June 9, 2024 01:35

f-lab-nathan approved these changes Jun 9, 2024

View reviewed changes

nosorae added 4 commits June 10, 2024 23:27

[CT-2-2-3] 리뷰반영 - dto의 ticks를 널 대신 빈 리스트로 초기화

c1231e7

[CT-2-2-3] 리뷰반영 - 차트의 tick 리스트가 부족할 때 그대로 보여준다.

8a07b8e

[CT-2-2-3] 리뷰반영 - 불필요한 소괄호 제거

3b02a4f

[CT-2-2-3] 리뷰반영 - 코드의 간결함을 위해 반복되는 코드는 확장함수등으로 만들기

d610fd1

f-lab-nathan merged commit 5298aeb into feature/ct-2-2-2-chart-game-presentation-screen Jun 11, 2024
2 checks passed

nosorae deleted the feature/ct-2-2-3-chart-game-bug-fix branch June 11, 2024 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CT-2-2-3] 차트게임 기능 - 실행 테스트 및 버그 수정 #35

[CT-2-2-3] 차트게임 기능 - 실행 테스트 및 버그 수정 #35

nosorae commented Jun 6, 2024 •

edited

Loading

nosorae Jun 6, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

nosorae Jun 10, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

f-lab-nathan Jun 9, 2024

nosorae Jun 10, 2024

nosorae commented Jun 10, 2024

sonarqubecloud bot commented Jun 10, 2024

[CT-2-2-3] 차트게임 기능 - 실행 테스트 및 버그 수정 #35

[CT-2-2-3] 차트게임 기능 - 실행 테스트 및 버그 수정 #35

Conversation

nosorae commented Jun 6, 2024 • edited Loading

Overview

깨닳은 점

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

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

nosorae commented Jun 10, 2024

sonarqubecloud bot commented Jun 10, 2024

Quality Gate passed

nosorae commented Jun 6, 2024 •

edited

Loading