Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 식당 정보 바텀 시트가 닫힌 직후 이미지 로딩이 진행될 때 발생할 수 있는 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
안녕하세요. 식당 정보 바텀시트에서 발생하는 Glide 크래시를 해결하기 위한 변경 사항을 리뷰했습니다. CoroutineScope를 viewLifecycleOwner.lifecycleScope로 변경하고, 뷰의 생명주기에 맞춰 바인딩 객체를 안전하게 처리하도록 수정한 점이 인상적입니다. 이로써 Fragment가 detach된 이후에 UI 업데이트를 시도하여 발생하던 NPE를 효과적으로 방지할 수 있을 것으로 보입니다. 한 가지 가독성 향상을 위한 제안을 리뷰 코멘트로 남겼으니 확인 부탁드립니다.
| val binding = _binding ?: return@launch | ||
|
|
||
| restaurantInfo?.let { | ||
| binding.tvLocation.text = it.location | ||
| binding.tvTime.text = it.time | ||
| binding.tvEtc.text = it.etc | ||
|
|
||
| Glide.with(this@InfoBottomSheetFragment) | ||
| Glide.with(binding.ivCafeteriaPhoto) | ||
| .load(it.image) | ||
| .into(binding.ivCafeteriaPhoto) | ||
| } |
There was a problem hiding this comment.
코루틴 내에서 binding이라는 이름의 지역 변수를 선언하여 사용하고 계십니다. 이 변수는 클래스 레벨의 binding 프로퍼티를 가리는(shadowing) 효과가 있어, 코드를 읽을 때 혼동을 줄 수 있습니다. 가독성 향상과 잠재적인 버그 방지를 위해, safeBinding과 같이 명확한 이름으로 변경하는 것을 제안합니다.
| val binding = _binding ?: return@launch | |
| restaurantInfo?.let { | |
| binding.tvLocation.text = it.location | |
| binding.tvTime.text = it.time | |
| binding.tvEtc.text = it.etc | |
| Glide.with(this@InfoBottomSheetFragment) | |
| Glide.with(binding.ivCafeteriaPhoto) | |
| .load(it.image) | |
| .into(binding.ivCafeteriaPhoto) | |
| } | |
| val safeBinding = _binding ?: return@launch | |
| restaurantInfo?.let { | |
| safeBinding.tvLocation.text = it.location | |
| safeBinding.tvTime.text = it.time | |
| safeBinding.tvEtc.text = it.etc | |
| Glide.with(safeBinding.ivCafeteriaPhoto) | |
| .load(it.image) | |
| .into(safeBinding.ivCafeteriaPhoto) | |
| } |
Summary
바텀 시트를 닫은 직후 식당 이미지 로드가 이어지면 Fragment가 detach된 상태에서
Glide.with(fragment)가 호출되어 NPE가 발생했어요.CoroutineScope(Dispatchers.Main)로 띄운 작업이 view lifecycle과 분리되어 있어서 BottomSheet가 닫힌 뒤에도 UI 업데이트가 이어질 수 있었어요.viewLifecycleOwner.lifecycleScope에 묶고, 뷰가 살아있을 때만 binding/Glide를 사용하도록 바꿨어요.Describe your changes
InfoBottomSheetFragment.ktCoroutineScope(Dispatchers.Main)를viewLifecycleOwner.lifecycleScope로 변경_binding이 null이면 바로 return 하도록 방어 로직 추가Glide.with(this@InfoBottomSheetFragment)대신Glide.with(binding.ivCafeteriaPhoto)로 요청을 view에 묶음onDestroyView()에서 binding 정리 추가Issue