타이머관련 리팩토링 및 최근앱에서 제거 해도 다시 반영되게 변경 #156
Conversation
# Conflicts: # main-presentation/src/main/java/com/teamhy2/feature/home/HomeScreen.kt # main-presentation/src/main/java/com/teamhy2/feature/home/HomeViewModel.kt # main-presentation/src/main/java/com/teamhy2/feature/home/model/HomeUiState.kt
main-presentation/src/main/java/com/teamhy2/feature/home/model/HomeUiState.kt
Show resolved
Hide resolved
timer-presentation/src/main/java/com/teamhy2/hongikyeolgong2/timer/presentation/model/Time.kt
Outdated
Show resolved
Hide resolved
| import java.time.LocalDateTime | ||
| import java.time.format.DateTimeFormatter | ||
|
|
||
| data class Time( |
There was a problem hiding this comment.
Time이라는 이름은 너무 일반적이고 추상적인 느낌이 강한 것 같습니다. 더 목적을 분명하게 TimerTime으로 변경하는 것은 어떠실까요?
| val hourAndMinute: String | ||
| get() = "$hour:$minute" |
There was a problem hiding this comment.
hourAndMinute 변수도 formattedTimerTime이라고 변경하는 것은 어떠할까요?
| fun create(endTime: LocalDateTime): LeftTime { | ||
| val now = LocalDateTime.now() | ||
| if (now.isAfter(endTime)) { | ||
| val timeOver = String.format(Locale.KOREA, LEFT_TIME_FORMAT, 0, 0, 0) |
There was a problem hiding this comment.
이 timeOver 값 자체도 상수화 가능해 보입니다
...a/src/main/java/com/teamhy2/hongikyeolgong2/timer/data/datastore/TimerDataStoreDataSource.kt
Show resolved
Hide resolved
| while (isTimeOver().not()) { | ||
| val leftSeconds: Long = leftTime.seconds |
There was a problem hiding this comment.
val leftSeconds: Long = leftTime.seconds
while(leftSeconds > Duration.ZERO){
}
이렇게 아래 로직과 통합하시는건 어떤가요?
There was a problem hiding this comment.
함수 사용처가 여기뿐이라 isNotTimeOver로 변경하였습니다.
| val durationMillis: Long = intent?.getLongExtra(EXTRA_TIME, 0L) ?: 0L | ||
| intent.getLongExtra(EXTRA_START_TIME, System.currentTimeMillis()) | ||
| val endTimeMillis: Long = | ||
| intent.getLongExtra(EXTRA_END_TIME, System.currentTimeMillis() + 1000 * 60 * 60 * 4) |
There was a problem hiding this comment.
1000 * 60 * 60 * 4 상수 자체는 난해한 감이 있는 것 같습니다. 적절한 상수화가 필요해 보입니다
| timerJob = | ||
| serviceScope.launch { | ||
| while (true) { | ||
| val remainingTime: Long = endTime - System.currentTimeMillis() | ||
| val leftTime = |
| appContext.startForegroundService(startIntent) | ||
| } | ||
|
|
||
| override fun stopService() { | ||
| val stopIntent = Intent(context, TimerForegroundService::class.java) | ||
| runBlocking { | ||
| withTimeout(4000L) { |
There was a problem hiding this comment.
withTimeout(4000L)에서 4초를 기준으로 잡으신 이유가 뭔가요?
There was a problem hiding this comment.
runBlocking을 사용한 이유부터 말씀드리자면, 서비스가 stop될 때, 공부시간을 저장하는 로직이 완료되지 않은 상태에서 종료되는걸 경계해서 사용하였습니다. (물론 서비스 스코프에서도 잘 돌아가기는 합니다. 혹시모를 상황을 대비하여 사용하였습니다.)
스레드를 블로킹함으로 ANR(5초동안 메인스레드 블락)의 위험성이 있기에 아무리 블로킹을 오래 하더라도 4초를 못넘기게 한 것입니다.
There was a problem hiding this comment.
ANR을 염두하고 한 것이 맞았군요 답변 감사합니다.
resolved #137
resolved #138
AS-IS
TO-BE
KEY-POINT
홈에서 각종 다이얼로그들이 떠있는지에 대한 상태까지 UIState로 다루는 것을 분리하였습니다.
프로모션 다이얼로그는 프로모션에 대한 데이터에 따라 다이얼로그를 표시할지 말지를 정함으로 뷰모델에서 관리하는게 맞지만, 다른 다이얼로그들은 HomeScreen 딴에서 자체적으로 처리가 가능하다 판단하여 분리하였습니다.
상태 호이스팅은 필요한 곳 까지만 올리는 것이 유리합니다.
QA용 앱은 메일로 보내드렸습니다 :)