-
Notifications
You must be signed in to change notification settings - Fork 47
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
[둘리] 1, 2단계 영화 극장 선택 제출합니다. #4
Conversation
[해시] 1, 2단계 영화 티켓 예매 제출합니다. (woowacourse#12) * docs: 기능 목록 작성 * feat: 영화 예매하는 기능 구현 * test(Minute): Minute 클래스에 대한 테스트 코드 생성 * feat(Reservation): 예매 생성 시 제약사항 구현 * docs: 기능 구현 목록 수정 - Movie의 띄어쓰기 수정 * feat(Reservation, Money): Reservation의 총 예매 금액 계산 구현 * refactor(Minute): Minute 클래스를 value class로 변경 * feat(MovieRepository): 영화 목 저장소 구현 * feat(MovieListActivity): 리스트뷰에 영화 목록 띄우는 기능 구현 * docs(MovieListActivity): 기능 목록 업데이트 * refactor: Activity 파일을 view 패키지로 옮김 * docs: 기능 목록 구현 상태 갱신 * refactor: layout 이름 변경 - activity_main -> activity_movie_list * feat(activity_reservation): 예약 액티비티의 레이아웃 구현 * feat(MovieListActivity): 영화 목록 화면에서 영화 예매 화면으로 이동 기능 구현 * feat(ReservationActivity): 예약 액티비티에서 받은 영화 데이터를 보여 주는 기능 구현 * fix(ReservationActivity): format 타입에 맞는 값으로 수정 * feat(ReservationCompletedActivity): 예약 완료 화면 레이아웃 구현 * feat(ReservationActivity): 영화 예매 화면에서 예매 완료 화면으로 이동 기능 구현 * docs(README): 기능 목록 업데이트 * style(MovieMockRepository): 목 데이터 오타 수정 * feat(ReservationCompletedActivity): 예매 완료 액티비티에서 받은 예매 데이터를 보여 주는 기능 구현 * refactor(strings): 포맷 문자열들의 아이디명을 변경 * feat(movie_item.xml): 영화 포스터 이미지 설명 추가 * docs: 기능 목록 구현 상태 갱신 * docs: 기능 목록 업데이트 * docs(README): 2단계 기능 목록 작성 * feat(Reservation, Movie): 상영 기간을 사용하도록 도메인 변경 * feat(activity_reservation.xml): 레이아웃에 스피너 추가 * feat(ReservationActivity): 영화 상영 시간 선택 기능 구현 * fix(Reservation, ReservationCompletedActivity): 예매한 상영 날짜, 시간 보여주도록 수정 * docs(README): 기능 목록 업데이트 * feat: 할인 정책 및 할인 조건 구현, 예매 시 할인 정책 적용 * feat(ReservationActivity): 화면 회전 시 상태 데이터 저장 및 복구 구현 * fix: 영화의 상영일의 포맷 변경 - yyyy-MM-dd -> yyyy.MM.dd * fix: LocalDate 타입을 시간 포멧을 지정하지 않도록 코드를 수정 * fix(ReservationActivity): 주말인 경우 time spinner 설정 및 회전 시 선택 된 시간 유지 * refactor(ReservationActivity): time spinner의 초기값을 0으로 수정 * fix(activity_reservation_completed.xml): 스크롤뷰로 수정 * refactor: 불필요한 도메인 클래스 제거 및 디미터 법칙 준수하도록 변경 * refactor(movie_item.xml): movie_item 뷰 배치 수정 * refactor(activity_reservation.xml): RelativeLayout -> ConstraintLayout으로 변경 * refactor(ReservationActivity): ViewBinding 사용하도록 수정 * refactor(ReservationCompletedActivity): ViewBinding 사용하도록 수정 * refactor(MovieListAdapter, MovieItemViewHolder): ViewHolder를 사용해 뷰 객체 캐싱 * refactor(Movie, ReservationUiModel): Parcelable 사용 및 예약 ui 모델 사용하도록 변경 * refactor(ReservationActivity): getParcelableExtra 코드 별도 메서드로 분리 * refactor(ReservationCompletedActivity): getParcelableExtra 코드 별도 메서드로 분리 * refactor(MovieListAdapter): 어댑터 context 제거 및 클릭 이벤트 외부에서 전달하도록 수정 * refactor(Movie): 상영기간에 해당하는 날짜 구하는 반복문 수정 * refactor(ReservationActivity): 메서드 분리 * refactor(ParcelableExtension): parcelable, serializable 버전 처리 코드 별도 extension으로 분리 * test(ReservationTest): 구체적인 테스트명으로 변경 * rename(DateTimeFormatter): DateTimeFormatter로 이름 변경 * refactor(Movie): 뷰에서 사용하는 MovieUiModel 분리 * refactor(ParcelableExtension): 메서드명 수정 --------- Co-authored-by: ki960213 <[email protected]> [해시] 3, 4 단계 영화 티켓 예매 제출합니다. (woowacourse#34) * refactor(ParcelableExtension): Suppress 어노테이션으로 warning 제거 * fix(MovieUiModel): 연,월이 다를 때 기간이 잘못 계산되는 문제 해결 * refactor: domain 모듈 분리 * docs(README): 기능 목록 업데이트 * docs(README): 좌석 관련 기능 목록 업데이트 * feat(SeatType): 좌석 타입 구현 * feat(Seat): 좌석 기능 구현 * refactor(DiscountPolicy, DiscountCondition): Reservation이 아닌 구체적인 값을 전달해 사용하도록 변경 * refactor(DiscountPolicy): 할인 비율, 할인 금액 각 DiscountPolicy가 갖도록 수정 * feat(ReservationAgency): 예매 가능한지 판단한다 * feat(ReservationAgency): 할인 정책 적용 금액 구하는 기능 및 예매하기 구현 * feat(ReservationActivity): SeatSelectionActivity로 이동 및 ReservationOptions 넘기도록 변경 * refactor(Reservation): 금액 관련 속성 제거 및 좌석을 받도록 수정 * refactor(ReservationUiModel): 좌석 정보 추가 * feat(SeatSelectionActivity): 좌석 선택 화면 구현 * fix(Seat): 좌석 행, 열 수정 * feat(SeatSelectionActivity): 예매 인원에 해당하는 좌석을 선택하고 금액 표시하는 기능 구현 * refactor: ktlintCheck * feat(SeatSelectionActivity): 예매 확인 다이얼로그 및 ReservationCompletedActivity로 이동 구현 * refactor(SeatSelectionActivity): 문자열 추출 * feat(ReservationCompletedActivity): 예매한 좌석 표시 * refactor: 파일 패키지 이동 * feat: 앱바 뒤로가기 구현 * test(SeatSelectionActivityTest): 좌석 선택 UI 테스트 * feat(MovieMockRepository): mock 데이터 추가 * refactor(MovieListActivity): listview를 recyclerview로 변경 * refactor: mapper 별도 파일로 분리 * feat(MovieListActivity): 영화 목록에 광고 추가 구현 * refactor(MovieListAdapter): bind 코드 ViewHolder로 이동 * refactor(MovieListAdapter): 클릭 이벤트 콜백 전달 및 호출 방식 수정 * test(ReservationActivityTest): ReservationActivity UI 테스트 * feat: 광고 배너 웹페이지 연결 * test(ReservationAgencyTest): 좌석 row column 수정 * feat(SeatSelectionActivity): 좌석 동적으로 생성하도록 변경 * refactor(SeatUiModel): 좌석 UI 모델 추가 * refactor: 패키지 변경 * refactor: ktlintFormat * refactor(MovieAdViewHolder, MovieItemViewHolder): 뷰홀더에서 하위 뷰 제거 * refactor(MovieListActivity): 광고 게시 간격 상수 분리 * refactor(MovieListAdapter, MovieListViewType): ViewType을 enum class로 관리 * refactor(SeatSelectionActivityTest): 테스트 함수명 수정 * refactor(ReservationActivityTest): 테스트 함수명 수정 * refactor(ReservationActivity): SeatSelectionActivity로 이동하는 코드 메서드 분리 * refactor(ReservationActivity, SeatSelectionActivity): 액티비티 진입에 필요한 Intent 생성 코드 이동 * refactor(ReservationCompletedActivity): 액티비티 진입에 필요한 Intent 생성 코드 이동 * refactor(ReservationActivity): 액티비티 진입에 필요한 Intent 생성 코드 이동 * test(SeatSelectionActivityTest, ReservationActivityTest): newIntent 메서드 사용해서 인텐트 생성하도록 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1, 2 단계 미션 잘 해주셨습니다.
드디어 도메인을 탈출한 소감이 어떠신가요?
더욱 즐거우신가요? 아니면 힘드신가요? 😱
안드로이드 프레임워크와 관련된 내용은 계속해서 익숙해져야 합니다. 화이팅
app/src/androidTest/java/woowacourse/movie/view/MovieMainActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/woowacourse/movie/view/MovieMainActivityTest.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/movie/view/seatselection/AlarmReceiver.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/movie/view/seatselection/SeatSelectionActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/movie/view/seatselection/SeatSelectionActivity.kt
Outdated
Show resolved
Hide resolved
도메인은 설계만 잘하면 되는 느낌이었는데 안드로이드를 들어오니 프레임워크를 제대로 공부해야겠다는 생각이 드네요 ㅎㅎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
피드백 반영 잘 해주셨습니다.
다음 단계에서 드디어 아키텍처에 관한 이야기를 다루게 될 테니 간단한 의견만 몇가지 남겨드렸습니다.
프래그먼트 관련해서 add와 replace에 대해서 다시 살펴보도록 내용을 같이 남겼습니다.
@Test | ||
fun 예매내역_버튼을_누르면_예매내역_Fragment로_바뀐다() { | ||
onView(withId(R.id.action_reservation_list)).perform(click()) | ||
onView(withId(R.id.recyclerview)).check(matches(isDisplayed())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프래그먼트가 보인다를 테스트해야 하는데, 실제로 해당 프래그먼트의 컨텐츠를 확인하는게 아쉽네요..
저도 프래그먼트 테스트는 자주 해보지 않아서, 어떤 방식이 최적의 방식인지는 추천드리기 어려운데
프래그먼트 매니저를 이용해서 프래그먼트의 인스턴스가 해당 타입이 맞는지 확인하는게 가능할까 싶네요!?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onView(withId(R.id.action_setting)).perform(click())
mActivityTestRule.scenario.onActivity {
val fragment = it.supportFragmentManager.findFragmentByTag(SettingFragment.TAG_SETTING)
assertTrue(fragment != null && fragment.isVisible)
}
TAG로 프래그먼트를 가져와서 그 프래그먼트가 visible인지 검사했습니다! :)
private fun setSharedPreferences(isAlarmOn: Boolean) { | ||
val targetContext: Context = InstrumentationRegistry.getInstrumentation().targetContext | ||
|
||
val preferencesEditor: SharedPreferences.Editor = PreferenceManager.getDefaultSharedPreferences(targetContext).edit() | ||
preferencesEditor.clear() | ||
preferencesEditor.putBoolean(SettingFragment.IS_ALARM_ON, isAlarmOn) | ||
preferencesEditor.commit() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래 내용은 꼭 반영하지 않으셔도 됩니다.
세팅 화면에서 UI 테스트를 하기 위해 프리퍼런스가 잘 동작하는지를 체크하는게 맞을까? 라는 의문이 들었어요.
만약 프리퍼런스가 아닌 데이터베이스로 바뀌면 UI 테스트가 같이 실패하게 됩니다.
객체지향 관점으로 접근해 보면, 어떤 방법으로 데이터를 가져오는지 관심 없도록 만들어 볼 수도 있겠네요
return Intent(context, AlarmReceiver::class.java).let { | ||
it.putExtra(AlarmReceiver.RESERVATION, reservation) | ||
PendingIntent.getBroadcast( | ||
context, | ||
ALARM_REQUEST_CODE, | ||
it, | ||
PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let에 대한 올바른 사용 방법이 아닙니다.
코틀린에서 제공하는 범위 함수를 어떤 상황에서 사용하는지는 조금씩 다르지만, 단순히 변수 줄이는 목적처럼 사용하지는 않습니다.
val pendingIntent = PendingIntent.getBroadcast( | ||
context, | ||
ALARM_REQUEST_CODE, | ||
Intent(context, AlarmReceiver::class.java), | ||
PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반복되는 코드가 보이면 어떻게 해야 한다!?
val reservation = intent.getParcelableCompat<ReservationUiModel>(RESERVATION) | ||
sharedPreferences = PreferenceManager.getDefaultSharedPreferences(this) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프리퍼런스를 가져오는 화면마다, 매번 동일한 코드를 이용해서 초기화를 해야 합니다.
그렇다면, 프리퍼런스 초기화를 비롯해 내부적으로 관리하는 객체를 만들어 보면 어떨까요?
private fun createChannel() { | ||
val name = "Reservation Notification" | ||
val channel = NotificationChannel( | ||
AlarmReceiver.CHANNEL_ID, | ||
name, | ||
NotificationManager.IMPORTANCE_DEFAULT, | ||
) | ||
val notificationManager = | ||
getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | ||
notificationManager.createNotificationChannel(channel) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
알림을 설정하고 채널을 관리하는 것도 알람 관련 객체에서 처리해야 하지 않을까요?
private fun replaceFragment(fragment: Fragment) { | ||
supportFragmentManager.commit { | ||
setReorderingAllowed(true) | ||
replace(R.id.fragment_container_view, fragment) | ||
} | ||
} | ||
|
||
private fun addFragment(fragment: Fragment, tag: String) { | ||
supportFragmentManager.commit { | ||
add(fragment, tag) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add와 replace는 어떤 차이가 있을까요?
add 이후에 replace를 하면 어떻게 동작을 할까요?
프래그먼트의 스택과 관련해서 로그를 찍어보면서 비교 해 보시면 좋습니다.
class MovieListFragment : Fragment(R.layout.fragment_movie_list) { | ||
override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
super.onViewCreated(view, savedInstanceState) | ||
val movies = MovieMockRepository.findAll() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MockRepository 다른 곳도 동일하게 변경 해주세요~
안녕하세요, 리뷰어님! 둘리입니다. 다시 뵙게 되어 반갑습니다.ㅎㅎ
이번 단계 변경사항은 다음과 같습니다. 링크
푸시 알림 테스트는 Espresso에서 불가능하여 하지 못했습니다. UI테스트가 많이 어렵네요...
날카로운 피드백 부탁드립니다! 감사합니다 :)