Open
Conversation
1 task
개요사운드 및 진동 피드백 시스템을 도입하여 볼륨 변경 및 알람 타입 선택 시 UI 상호작용에 대한 오디오/촉각 피드백을 추가합니다. 새로운 변경사항
주요 검토 포인트성능 및 리소스 관리:
안정성:
아키텍처:
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~20분 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/depromeet/team6/presentation/ui/mypage/SoundSamplePlayer.kt`:
- Around line 38-52: playVolumeSample currently calls toneGenerator.release()
and recreates a new ToneGenerator on every call, causing heavy resource churn
when the slider fires many events; change this to reuse a single ToneGenerator
instance (toneGenerator) and update its volume instead of releasing/recreating,
or implement a short debounce on playVolumeSample to limit calls during slider
drags. Specifically, keep a single ToneGenerator created lazily (use the
existing toneGenerator field), avoid calling release() on each invocation, and
only call startTone(ToneGenerator.TONE_PROP_BEEP, 120) when volume actually
changes after debounce; ensure any final cleanup still calls
toneGenerator.release() when the component is destroyed.
- Around line 1-2: SoundSamplePlayer is placed under presentation.ui.mypage but
is reused by onboarding; move the class to a shared package (e.g.,
presentation.ui.common or presentation.util) by changing its package declaration
in SoundSamplePlayer and updating all import sites (references in onboarding and
mypage code) to the new package; ensure any visibility modifiers remain
public/internal as appropriate and run a project-wide import/compile fix to
update imports and resolve references to SoundSamplePlayer.
- Around line 17-21: getRingtone(...) can return null so storing it as a
non-null Ringtone causes NPEs when you later call notificationRingtone.isPlaying
or notificationRingtone.play(); change the field to a nullable type
(notificationRingtone: Ringtone?), assign the result of
RingtoneManager.getRingtone(...) directly, and everywhere you use
notificationRingtone (e.g., in play/stop methods in SoundSamplePlayer) add null
checks or safe calls (?.isPlaying, ?.play(), ?.stop()) or provide a fallback
no-op behavior so calls are safe when getRingtone() returned null.
Comment on lines
+1
to
+2
| package com.depromeet.team6.presentation.ui.mypage | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
SoundSamplePlayer 패키지 위치 재검토 필요
SoundSamplePlayer가 mypage 패키지에 위치하지만 onboarding에서도 사용됩니다. 재사용 가능한 유틸리티 클래스이므로 common 또는 util 패키지로 이동하는 것이 아키텍처적으로 적절합니다.
Also applies to: 13-15
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/depromeet/team6/presentation/ui/mypage/SoundSamplePlayer.kt`
around lines 1 - 2, SoundSamplePlayer is placed under presentation.ui.mypage but
is reused by onboarding; move the class to a shared package (e.g.,
presentation.ui.common or presentation.util) by changing its package declaration
in SoundSamplePlayer and updating all import sites (references in onboarding and
mypage code) to the new package; ensure any visibility modifiers remain
public/internal as appropriate and run a project-wide import/compile fix to
update imports and resolve references to SoundSamplePlayer.
Comment on lines
+17
to
+21
| private val notificationRingtone: Ringtone = | ||
| RingtoneManager.getRingtone( | ||
| context, | ||
| RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION) | ||
| ) |
There was a problem hiding this comment.
RingtoneManager.getRingtone()은 null을 반환할 수 있음
기본 알림음이 설정되지 않은 기기에서 getRingtone()이 null을 반환하면 이후 notificationRingtone.isPlaying 및 play() 호출 시 NPE가 발생합니다.
🛡️ Null 안전성 처리 제안
- private val notificationRingtone: Ringtone =
- RingtoneManager.getRingtone(
+ private val notificationRingtone: Ringtone? =
+ RingtoneManager.getRingtone(
context,
RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION)
)
fun playNotificationSample() {
- if (notificationRingtone.isPlaying) {
- notificationRingtone.stop()
+ notificationRingtone?.let { ringtone ->
+ if (ringtone.isPlaying) {
+ ringtone.stop()
+ }
+ ringtone.play()
}
- notificationRingtone.play()
}
fun release() {
toneGenerator.release()
- if (notificationRingtone.isPlaying) {
- notificationRingtone.stop()
- }
+ notificationRingtone?.takeIf { it.isPlaying }?.stop()
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/depromeet/team6/presentation/ui/mypage/SoundSamplePlayer.kt`
around lines 17 - 21, getRingtone(...) can return null so storing it as a
non-null Ringtone causes NPEs when you later call notificationRingtone.isPlaying
or notificationRingtone.play(); change the field to a nullable type
(notificationRingtone: Ringtone?), assign the result of
RingtoneManager.getRingtone(...) directly, and everywhere you use
notificationRingtone (e.g., in play/stop methods in SoundSamplePlayer) add null
checks or safe calls (?.isPlaying, ?.play(), ?.stop()) or provide a fallback
no-op behavior so calls are safe when getRingtone() returned null.
Comment on lines
+38
to
+52
| fun playVolumeSample(volume: Float) { | ||
| val scaled = (kotlin.math.sqrt(volume) * 100) | ||
| .coerceAtLeast(10f) | ||
| .toInt() | ||
|
|
||
| // 이전 톤 정리 | ||
| toneGenerator.release() | ||
|
|
||
| toneGenerator = ToneGenerator( | ||
| AudioManager.STREAM_NOTIFICATION, | ||
| scaled | ||
| ).apply { | ||
| startTone(ToneGenerator.TONE_PROP_BEEP, 120) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ToneGenerator 빈번한 재생성으로 인한 리소스 부하
슬라이더를 드래그할 때마다 ToneGenerator를 release() 후 재생성하는 방식은 리소스 낭비가 될 수 있습니다. 슬라이더 드래그 중 수십 번 호출될 수 있으므로, 디바운싱을 고려하거나 단일 인스턴스를 재사용하는 방식을 검토해 보세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/depromeet/team6/presentation/ui/mypage/SoundSamplePlayer.kt`
around lines 38 - 52, playVolumeSample currently calls toneGenerator.release()
and recreates a new ToneGenerator on every call, causing heavy resource churn
when the slider fires many events; change this to reuse a single ToneGenerator
instance (toneGenerator) and update its volume instead of releasing/recreating,
or implement a short debounce on playVolumeSample to limit calls during slider
drags. Specifically, keep a single ToneGenerator created lazily (use the
existing toneGenerator field), avoid calling release() on each invocation, and
only call startTone(ToneGenerator.TONE_PROP_BEEP, 120) when volume actually
changes after debounce; ensure any final cleanup still calls
toneGenerator.release() when the component is destroyed.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 발행 전 체크 리스트
스크린샷 (선택)
💬리뷰 요구사항(선택)
리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
릴리스 노트