[수달] 3, 4단계 영화 극장 선택 제출합니다. #39
Conversation
- step1을 진행하며 이해하지 못한 코드들을 제거하고 다시 만들기 위함
- interface 적용 - 메서드, 변수명 등의 이름 개선
- MovieListAdapter, ReservationListAdapter 각각으로 사용하던 것을 CommonListAdapter로 통합
- 예약 확인 화면에서 할인을 적용하는 문제 해결하기 (추후 presenter를 사용하지 않도록 수정하기)
app/src/main/java/woowacourse/movie/data/setting/MovieReminderSetting.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/movie/data/setting/AlarmSetting.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/movie/data/setting/MovieReminderSetting.kt
Outdated
Show resolved
Hide resolved
| import woowacourse.movie.feature.common.viewHolder.MovieViewHolder | ||
| import woowacourse.movie.feature.common.viewHolder.TicketsViewHolder | ||
|
|
||
| class CommonListAdapter( |
There was a problem hiding this comment.
영화와 함께 광고도 담고 있어 어떤 이름으로 해야할지 몰라 공통적으로 쓰겟다는 의미였습니다.
하지만 주된 기능은 영화를 연결해 주는 것이기 때문에 MovieListAdapter로 수정하였습니다!
app/src/main/java/woowacourse/movie/feature/movielist/MovieListContract.kt
Outdated
Show resolved
Hide resolved
| @@ -11,29 +9,30 @@ import java.time.LocalDateTime | |||
| import java.time.LocalTime | |||
|
|
|||
| class DateTimeSpinner( | |||
| view: View, | |||
| private val binding: ActivityMovieDetailBinding, | |||
There was a problem hiding this comment.
우선, 기본적으로 안드로이드에서 말하는 위젯 개념은 외부의 View를 수정하는 역할이 아닌 독립적으로 UI를 그릴 수 있는 역할을 가져야 합니다.
즉 위 객체는 이렇게도 사용을 할 수 있어야 합니다
<woowacourse.movie.feature.reservation.reserve.selection.DateTimeSpinner
android:layout_width="..."
... />위젯 개념이 아닌 객체로 바라본다면 DateTimeSpinner는 내부적으로 지속해서 binding의 상태를 바꾸는 이펙트를 유발하게 됩니다.
결국 뷰에서 private 함수로 가지고 있어도 되기에 클래스로 분리된 장점이 없다가 됩니다. (재사용이 안되요)
이 내용은 이해가 지금은 잘 되지 않으실 거에요.
안드로이드를 조금 더 학습하시면서 커스텀 뷰를 만들 수 있을 때 다시 고민해보셔도 됩니다.
| val hasPermission: Boolean = hasPermission(requireActivity(), Manifest.permission.POST_NOTIFICATIONS) | ||
| presenter.setMovieReminderChecked(hasPermission) |
There was a problem hiding this comment.
대부분의 화면에서 onResume()이 무슨 동작을 하는 것일까요?
데이터를 수정하는 것 같은데 어떠한 의도인지 궁금합니다
There was a problem hiding this comment.
앱 내부 설정의 권한이 아닌 안드로이드의 앱 설정에서 권한이 꺼졌을 경우를 생각해 onResume()에서 매번 확인하고, 설정하도록 하였습니다.
| override fun setMovieReminderChecked(hasPermission: Boolean) { | ||
| view.setMovieReminderChecked(userSetting.enabled) | ||
|
|
||
| if (!hasPermission) { | ||
| userSetting.enabled = false | ||
| view.setMovieReminderChecked(false) | ||
| view.requestPermission() | ||
| } | ||
| } |
There was a problem hiding this comment.
지금 로직으로 보면, 프레젠터에서 hasPermission을 받을 이유가 있을까요?
| @Before | ||
| fun setUp() { | ||
| alarmSetting = mockk() | ||
| view = mockk() |
There was a problem hiding this comment.
mockk(relaxed = true) 형태로 생성하면 just runs 등을 생략할 수 있습니다.
| @Before | ||
| fun setUp() { | ||
| alarmSetting = mockk() | ||
| view = mockk() |
There was a problem hiding this comment.
mockk(relaxed = true) 형태로 생성하면 just runs 등을 생략할 수 있습니다.
안녕하세요 페로로님!
제목은 영화 극장 선택 제출합니다 이지만, 정작 극장 선택 기능은 구현하지 못했네요...
페어의 코드를 이해하고, 리펙터링을 하고, MVP를 적용하는 것만으로 조금 벅찼던 것 같습니다.
MVP도 모두 적용하지는 못했지만, 적용한 부분까지만이라도 피드백을 받고 싶어 리뷰요청드립니다.
또한, 이번에는 이전 미션 중 일부를 재구현하였습니다.
재구현한 부분을 제외한 커밋 내역입니다!
그리고 이전에 남겨주신 피드백 또한 도움이 많이 되었습니다! 감사합니다!
이전에는 싱글톤을 위해 object만을 사용하였습니다. 그러나 이런 경우 유지보수를 하며 object는 사용되지 않아 문제가 되기도 한다는 말을 들은적이 있습니다.
페로로님께서 저번 리뷰에 companion object를 활용해 싱글톤으로 관리하는 방법을 알아보라고 추천해주셔서 사용해보기도 하였습니다! 이전 방법의 단점이 개선된 방법을 연습해볼 수 있어 좋았습니다!
그러나 prefernce와 같은 경우에는 context가 필요합니다. context를 가지는 싱글톤의 객체가 activity context를 가지고 있을 경우 해당 activity가 사라지더라도 prefernce를 관리하는 객체에서 참조하고 있기 때문에 GC 대상이 되지 않아 메모리 누수가 발생합니다.
application context를 사용한다면 문제가 되지 않겠지만 application context를 강제하거나 application context인지 확인할 방법 등이 있는 것 같지 않습니다. 단순히 메모리 누수에 대한 걱정으로 application context를 쓰는 방법이 좋은 방법일까? 에 대한 고민이 있습니다…!
아직 이 부분에 대해 해결 방법이 떠오르지 않아 우선 context를 받아 초기화하도록 수정하였습니다. 이 경우에는 어떤 더 좋은 방법이 있을지도 궁금합니다!