Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/180 timer service 적용 #184

Merged
merged 7 commits into from
Aug 4, 2023
Merged

Conversation

unam98
Copy link
Contributor

@unam98 unam98 commented Aug 2, 2023

📌 개요

#180 service를 활용한 timer 이슈 해결

장시간 앱 이탈, 잠금화면 시 timer가 시간 반영이 제대로 안 된다거나 초기화 된다는 등의 문제를 보고 받았습니다. 기존에 백그라운드 동작에 대한 별 다른 조취가 없었던 것을 원인으로 파악하고 서비스를 활용하여 대응 작업을 해주었습니다.

<안드로이드-서비스-공식문서>
https://developer.android.com/guide/components/services?hl=ko

✨ 작업 내용

  • timer 관련 logic을 TimerService로 분리
  • memory leak이 발생하지 않게 생명주기를 고려하여 service 제거 코드 작성

🧐 리뷰 포인트

서비스는 백그라운드에서 계속 도는 만큼 memory leak에 더욱 유의해야 한다고 생각하는데 서비스 사용이 처음이라 제가 중요한 부분을 놓쳤을 수 있습니다. 서비스 객체의 생성과 제거, 생명주기 쪽에서 안정적으로 잘 작업했는지 봐주시면 좋을 것 같습니다!

잠금화면 등 백그라운드에서 장시간 작업이 계속 돌 수 있게 서비스 클래스를 생성해주었음
단순히 UI 바인딩 목적이면 시/분/초를 문자열 포맷팅을 활용해서 하나의 값으로 다룰 수 있는데 EndRunActivity에서 평균 페이스 계산할 때 시/분/초 각각의 값이 모두 필요함. 

따라서 시/분/초가 모두 합쳐진 String 값 하나만 보내면 복잡해지므로 이것들만 따로 빼서 보낼 수 있게 data class를 만들어주었음
- timer 관련 logic은 TimerService로 분리했음 (서비스 시작 시 호출되는 onStartCommand() 블록 안에 작성함)

- 이러한 서비스를 액티비티와 연결시켜주기 위해 startService(), bindService()를 해주었고 러닝 종료 시 unBindService()와 stopService()를 해주어 memory leak이 일어나지 않게 해주었습니다.
- data binding으로 뷰모델에 정의해놓은 LiveData를 xml에서 구독하고 있는데 총거리값을 LiveData에 할당 안 해줘서 반영이 안 되고 있었음.

- 반올림 확장함수 생성
@unam98 unam98 requested a review from Larry7939 August 2, 2023 18:50
@unam98 unam98 closed this Aug 2, 2023
리팩토링하는 과정에서 intent로 넣어줘야 할 data의 세팅 코드를 지워버려서 에러가 났고 현재는 해결해주었음.
@unam98 unam98 reopened this Aug 2, 2023
@unam98 unam98 added the 우남 🐼 우남 담당 label Aug 4, 2023
Copy link
Contributor

@Larry7939 Larry7939 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생 많았습니다~!

stopTimer()
finish()
}

private fun backButton() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이미 onDestroy에서 stopTimer를 호출하고 있기 때문에, backButton()에서 stopTimer()를 중복 호출해줄 필요는 없을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그게 좋겠네요 반영하겠습니다 확인 감사합니다!

stopTimer()
finish()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이미 finish()가 호출된 후에 onDestroy가 호출되는 것이므로, 불필요한 중복 호출인 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러네요. 반영하겠습니다! 확인 감사합니다!

Comment on lines -257 to +319
timerHour = timerHour,
timerMinute = timerMinute,
timerSecond = timerSecond,
timerHour = timerData.hour,
timerMinute = timerData.minute,
timerSecond = timerData.second,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finish() 호출이 누락된 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가하겠습니다 확인 감사합니다!

Comment on lines +34 to +50
val hour = time / 3600
val minute = time / 60
val second = time % 60

val timerValue = String.format("%02d:%02d:%02d", hour, minute, second)
Timber.tag("Timer").d(timerValue)

// 결과를 Activity로 전달
val intent = Intent(TIMER_UPDATE_ACTION)
intent.putExtra(
EXTRA_TIMER_VALUE, TimerData(
hour = hour,
minute = minute,
second = second
)
)
sendBroadcast(intent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timerValue는 삭제해도 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기능 상 필요한 부분은 아닌데 서비스가 제대로 종료 안 됐거나 객체가 여러개 생성돼서 memory leak이 일어날 경우 log가 아니면 즉각적으로 파악이 어려워서 모니터링하려고 만들어준 부분입니다!

Comment on lines 64 to 65
const val TIMER_UPDATE_ACTION = "com.runnect.runnect.TIMER_UPDATE_ACTION"
const val EXTRA_TIMER_VALUE = "com.runnect.runnect.EXTRA_TIMER_VALUE"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앞에 패키지 명이 들어간 이유가 있을까요?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

특별한 이유는 없습니다!

- 불필요한 stopTimer() 호출 제거
- EndRunActivity로 넘어갈 때 finish() 추가
- 불필요한 finish() 삭제
@unam98 unam98 merged commit fb2adce into develop Aug 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
우남 🐼 우남 담당
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants