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

[둘리] 1, 2단계 영화 극장 선택 제출합니다. #4

Merged
merged 32 commits into from
Apr 29, 2023

Conversation

hyemdooly
Copy link

@hyemdooly hyemdooly commented Apr 27, 2023

안녕하세요, 리뷰어님! 둘리입니다. 다시 뵙게 되어 반갑습니다.ㅎㅎ

이번 단계 변경사항은 다음과 같습니다. 링크
푸시 알림 테스트는 Espresso에서 불가능하여 하지 못했습니다. UI테스트가 많이 어렵네요...
날카로운 피드백 부탁드립니다! 감사합니다 :)

woowahan-pjs and others added 13 commits April 25, 2023 14:39
[해시] 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 메서드 사용해서 인텐트 생성하도록 수정
Copy link

@laco-dev laco-dev left a comment

Choose a reason for hiding this comment

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

1, 2 단계 미션 잘 해주셨습니다.

드디어 도메인을 탈출한 소감이 어떠신가요?
더욱 즐거우신가요? 아니면 힘드신가요? 😱

안드로이드 프레임워크와 관련된 내용은 계속해서 익숙해져야 합니다. 화이팅

README.md Outdated Show resolved Hide resolved
app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/reservation_item.xml Outdated Show resolved Hide resolved
@hyemdooly
Copy link
Author

도메인은 설계만 잘하면 되는 느낌이었는데 안드로이드를 들어오니 프레임워크를 제대로 공부해야겠다는 생각이 드네요 ㅎㅎ
누군가 만들어둔 것을 활용하는 것도 쉬운 일이 아닌 것 같습니다!
열심히 공부해보겠습니다 :)

Copy link

@laco-dev laco-dev left a 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()))

Choose a reason for hiding this comment

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

프래그먼트가 보인다를 테스트해야 하는데, 실제로 해당 프래그먼트의 컨텐츠를 확인하는게 아쉽네요..

저도 프래그먼트 테스트는 자주 해보지 않아서, 어떤 방식이 최적의 방식인지는 추천드리기 어려운데
프래그먼트 매니저를 이용해서 프래그먼트의 인스턴스가 해당 타입이 맞는지 확인하는게 가능할까 싶네요!?

Copy link
Author

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인지 검사했습니다! :)

Comment on lines +53 to +60
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()
}

Choose a reason for hiding this comment

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

아래 내용은 꼭 반영하지 않으셔도 됩니다.

세팅 화면에서 UI 테스트를 하기 위해 프리퍼런스가 잘 동작하는지를 체크하는게 맞을까? 라는 의문이 들었어요.

만약 프리퍼런스가 아닌 데이터베이스로 바뀌면 UI 테스트가 같이 실패하게 됩니다.
객체지향 관점으로 접근해 보면, 어떤 방법으로 데이터를 가져오는지 관심 없도록 만들어 볼 수도 있겠네요

Comment on lines +35 to +43
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,
)
}

Choose a reason for hiding this comment

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

let에 대한 올바른 사용 방법이 아닙니다.

코틀린에서 제공하는 범위 함수를 어떤 상황에서 사용하는지는 조금씩 다르지만, 단순히 변수 줄이는 목적처럼 사용하지는 않습니다.

https://kotlinlang.org/docs/scope-functions.html

Comment on lines +47 to +52
val pendingIntent = PendingIntent.getBroadcast(
context,
ALARM_REQUEST_CODE,
Intent(context, AlarmReceiver::class.java),
PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT
)

Choose a reason for hiding this comment

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

반복되는 코드가 보이면 어떻게 해야 한다!?

Comment on lines +39 to +40
val reservation = intent.getParcelableCompat<ReservationUiModel>(RESERVATION)
sharedPreferences = PreferenceManager.getDefaultSharedPreferences(this)

Choose a reason for hiding this comment

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

프리퍼런스를 가져오는 화면마다, 매번 동일한 코드를 이용해서 초기화를 해야 합니다.

그렇다면, 프리퍼런스 초기화를 비롯해 내부적으로 관리하는 객체를 만들어 보면 어떨까요?

Comment on lines +99 to +109
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)
}

Choose a reason for hiding this comment

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

알림을 설정하고 채널을 관리하는 것도 알람 관련 객체에서 처리해야 하지 않을까요?

Comment on lines +43 to +54
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)
}
}
Copy link

@laco-dev laco-dev Apr 29, 2023

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()

Choose a reason for hiding this comment

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

MockRepository 다른 곳도 동일하게 변경 해주세요~

@laco-dev laco-dev merged commit 15fac5e into woowacourse:hyemdooly Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants