[우아한테크코스] 안드로이드 레벨2 영화 극장 선택 미션 회고

 

미션 관련 링크

영화 극장 선택 레포지토리

https://github.com/kimhm0728/android-movie-theater
영화 극장 선택 1, 2단계 PR

https://github.com/woowacourse/android-movie-theater/pull/57

영화 극장 선택 3, 4단계 PR
https://github.com/woowacourse/android-movie-theater/pull/75

 

 

 

코드 리뷰 받은 코멘트들 

Util 패키지에 Model -> String으로 변환하는 Formatter 함수 vs 화면 별 UiModel

내 코드에는 util 패키지에 모델을 화면에 띄우는 문자열로 변환하는 함수를 구현하고, 이를 전역적으로 쓰는 현태였다.

하지만 어떤 모델을 출력할 때, 모든 화면에서 같은 형식으로 출력된다는 것을 보장할 수 없다.

만약 이 전역적인 함수를 모든 화면에서 사용하다가, 하나의 화면만 출력 형식이 바뀐다면 수정하기 까다로울 것이다.

 

그래서 나는 화면 별로 UiModel을 만드는 것을 선호했는데, 이 경우 코드의 양이 많아진다.

이 부분에 대해 리뷰어님의 생각이 궁금해서 질문을 드렸다.

 

화면 별로 출력 형식이 다르다면 화면 별 UiModel을 만들겠지만,

그렇지 않다면 기획자/스펙 담당자와 논의하여 스펙 형식을 통일시키는 것도 하나의 방법이 될 수 있다.

좋은 구조를 위해 꼭 개발적으로만 접근할 필요는 없다.

아직은 많이 와닿지 않지만.. 현업에서 이 코멘트를 떠올려도 좋을 것 같음.

 

 

리스너를 고차 함수로 전달하기 vs 리스너 인터페이스를 생성 후 구현체로 전달하기

나는 후자를 선호했다. 인터페이스가 하나의 명세 역할도 하고, 고차 함수는 가독성이 떨어지기 때문이다.
또한 databinding을 통해 리스너를 등록한다면, binding 객체에 구현체 타입을 지정해줄 수 있다.

 

그런데 리뷰어님은 전자의 방식을 선호했다. 이유는 아래와 같다.

onClickMovie: (movieId: Long) -> Unit)

typealias OnClickMovie = (movieId: Long) -> Unit

먼저, 고차 함수가 가독성이 떨어진다는 부분은 위 코드처럼 typealias 또한 함수 인자에 이름을 붙여서 해결할 수 있다.

기본적으로 인터페이스는 다형성을 위해 사용되고, 함수가 일급 객체가 아닌 자바에서는 SAM을 사용해 리스너를 구현했다.

하지만 코틀린은 함수가 일급 객체이므로 리스너는 일반적으로 고차 함수로 전달한다.

 

그런데 인터페이스를 선호하는 리뷰어님도 계신 것 같다. 이 부분은 완전한 취향 차이인듯!
각자의 장단점을 짚고 넘어가면 좋을 것 같아 정리했다.

 

 

parentFragmentManager와 childFragmentManager를 넘겼을 때의 차이

각각의 프래그먼트는 자기 자신만의 FragmentManager (또는 백스택)을 가지고 있다.

자신의 백스택에 새로운 프래그먼트를 시작할 때는 childFragmentManager를 사용하고,

자신을 호출한 상위 액티비티/프래그먼트의 백스택에 새로운 프래그먼트를 시작할 때는 parentFragmentManager를 사용한다.

 

 

프래그먼트 트랜잭션 내부에 넣어준 setReorderingAllowed(true) 옵션의 의미

프래그먼트를 전환하는 여러 트랜잭션이 중복될 경우, 불필요한 작업이 발생할 수 있다.

예를 들어 "프래그먼트 A를 띄우는 트랜잭션"이 실행된 후, 바로 "프래그먼트 B를 띄우는 트랜잭션"이 중복되어 실행되는 경우, 프래그먼트 A와 B의 생명주기 모두 호출된다.

하지만 프래그먼트 B만 화면에 띄우면 되므로, 프래그먼트 A의 생명주기는 호출될 필요가 없다.

이때 setReorderingAllowed(true)를 지정해주면, 프래그먼트 전환 시 작업들을 최적화해주기 때문에 프래그먼트 A의 생명주기는 호출되지 않고 프래그먼트 B의 생명주기만 호출된다.

 

사실 강의 자료 코드를 따라치다보니, setReorderingAllowed 의 의미를 제대로 알지 못하고 사용했다.

이 코드에 대해 코멘트가 달린 이후 급하게 찾아보긴 했지만,

외부에서 코드를 가져오는 경우 해당 코드가 어떤 기능을 하는지 가볍게 찾아보는 습관을 들여야겠다..

 

 

프래그먼트를 백스택에 추가할 때, tag를 지정해주기

프래그먼트의 tag를 통해 특정 프래그먼트를 조회하는 등의 다양한 작업을 할 수 있다. tag를 붙이는 습관을 들이면 좋다.

나는 하단 네비게이션 바를 구현하면서, 다른 메뉴를 누를 때마다 계속 새로운 프래그먼트 인스턴스가 생성되는 것이 어색했다.

 

그래서 이 프래그먼트 tag를 이용해서 한 번만 add(인스턴스 생성)해주고, 이후에는 그 인스턴스를 다시 show/hide하도록 수정했다.

이미 인스턴스가 생성되었는지의 여부를 판단하고, 생성되었다면 그 인스턴스를 가져올 때 tag를 활용할 수 있었다.

하지만 replace가 아닌 show/hide 방식은 호출되는 lifecycle이 다르기 때문에 데이터를 로드하는 시점에 신경써야 한다. 

참고 링크 : https://pluu.github.io/blog/android/2023/01/19/fragment_visible_lifecycleowner/

 

 

try/catch와 runCatching

• 내 의견!

    runCatching {
        MovieRepository.getMovieById(movieId)
    }.getOrElse {
        view.handleInvalidMovieIdError(it)
        return
    }
    // 어쩌구 저쩌구 코드들
    // ...
    // ...

예외가 발생하는 부분만 runCatcing으로 감싸서 의미를 명확히 한다.

모든 코드를 try 블럭 안으로 감싸면, 어떤 부분에서 예외가 터지는지 알 수 없고 depth가 깊어진다.

 

• 리뷰어님의 의견!

try {
     val movie = MovieRepository.getMovieById(movieId)
     // 어쩌구 저쩌구 코드들
     // ...
     // ...
} catch (exception: Exception) {
     view.handleInvalidMovieIdError(exception)
}

내 방식은 "예외가 발생할 수 있는 부분이 한 군데"인 경우만 유용하다.

만약 예외가 두 곳 이상에서 발생한다면, runCatching이 여러번 쓰여 오히려 복잡해질 것이다.

또한 try/catch는 일종의 트랜잭션 개념이 포함되어 있다.

예외가 어디서 발생할지 모르지만, 예외 이후의 코드는 실행되지 않는다는 것을 드러낼 수 있다.

 

 

application 내부에 상태를 선언하는 것은 신중하게 결정한다

싱글톤으로 선언되어야할 객체를 application에서 초기화하고, application의 상태로 가지고 있었다.

하지만 application의 상태는 아래의 이유로 매우 신중하게 추가해야 한다.

• 전역적으로 참조가 가능한 상태는 안티 패턴이며 보통 전역적으로 쓰이지 않는다.
• 앱의 전체 수명주기에 관여하기 때문에, 무거운 작업을 수행하거나 에러를 트래킹할 때 적절하지 않다.

application context가 필요하다면, 객체가 필요한 부분에 application context를 직접 참조하자.

 

 

백그라운드 스레드 작업 관련한 시행착오들 🫠

* 이번 미션에서는 코루틴을 사용하지 않았다. 스레드만 사용해서 비동기 작업을 제어했어야 했다.

 

    override fun loadTickets(ticketRepository: TicketRepository) {
        Thread {
            tickets = ticketRepository.findAll()
            view.displayTickets(tickets)
        }
    }

 

가장 처음에 작성했던 Presenter 코드다.

view.displayTickets()는 ui 변경을 수행하는 함수다.

즉, ui 변경을 ui 스레드가 아닌 백그라운드 스레드에서 수행하고 있으므로 오류가 발생했다.

 

오류를 해결하려면 실제 view의 displayTickets()에서 runOnUiThread {}를 호출해야 한다.

이 방식은 presenter가 백그라운드 스레드 작업을 하고 있다는 것을 알고 있는 것이므로, 적절하지 않다고 생각했다.

 

    override fun loadTickets(ticketRepository: TicketRepository) {
        val handler = Handler(Looper.getMainLooper())
        Thread {
            val tickets = ticketRepository.findAll()
            handler.post { view.displayTickets(tickets) }
        }
    }

두 번째로 작성했던 Presenter 코드다.

백그라운드 스레드에서 레포지토리에 접근하여 데이터를 가져오고, 메인 스레드의 핸들러에 알려준다.

이 방식의 경우 Presenter 테스트가 어려워진다.

view.displayTickets()가 호출되었는지 테스트할 수 없었다. (테스트 방법이 있을 수는 있겠지만, 찾지 못했다..)

 

    override fun loadTickets(ticketRepository: TicketRepository) {
        var tickets = emptyList<Ticket>()
        thread {
            tickets = ticketRepository.findAll()
        }.join()
        view.displayTickets(tickets)
    }

최종 Presenter 코드다.

thread {} 외부에서 tickets를 var로 선언하고, thread {} 내부에서 불러온 실제 데이터를 대입해준다.

그리고 스레드 작업이 끝날 때까지 기다리고, view.displayTickets()를 호출한다.

 

아직 비동기 프로그래밍을 제대로 배우지 않은 상태에서는.. 최선의 방법이라고 생각한다.

하지만 스레드 작업이 끝날 때까지 현재 스레드를 blocking하기 때문에 비동기적인 방법은 아니다.

만약 스레드 작업이 10초라면, 현재 스레드(ui 스레드)가 멈춰있어서 사용자 측면에서 좋지 않을 것이다.

 

 

presenter가 context를 알아도 될까?

이 코멘트와 관련해서는 하고 싶은 말이 많아서, 따로 포스팅했다.

https://thdbs523.tistory.com/418

 

 

리뷰어/페어 피드백

리뷰어 피드백

- 본인의 생각을 논리적으로 정리하고, 이를 잘 풀어내는 모습이 좋았음.

- 무조건적으로 리뷰 내용을 수용하는 것이 아닌, 이해가 되는 부분과 그렇지 않은 부분을 인지하고 대화를 이어나가는 부분이 인상깊었음.

- 커뮤니케이션 과정에서 미스가 없도록 본인의 의견을 상세하게 남겨주어 좋았음.

 

페어 피드백

- 코드를 이해하는 속도가 빠름.

- 검색을 통해 알게 된 지식을 빠르게 자신의 코드에 적용하는 능력이 뛰어났음.

- 의견을 잘 들어주고 적극적으로 수용하려 해줌.

- 자신이 원하는 의도를 이해하기 쉽도록 설명을 해줌.

 

 

영화 극장 선택 미션을 끝내고

당시에는 무난하게 지났던 미션이었다고 생각했는데,

미션을 마친 후 한 달 정도가 지난 시점에서 다시 돌이켜보니 정말 얻은게 많은 미션이었다.

 

일단 처음으로 다른 크루의 코드를 기반으로 미션을 진행해봤다.

다른 크루의 코드를 하나하나 해석하면서 새롭게 배운 점도 있고, '아.. 이 부분은 다른 방법이 더 깔끔한데'라는 생각을 하기도 했다.

우테코에서 여러 미션을 진행하면서 내 주관이 생긴 결과가 아닐까 싶다.

 

사실 처음에는 이미 알고 있는 부분인데도 코드 리뷰를 받고, 다른 사람의 코드를 리팩터링하는 것이 조금 힘들었다.

그런데 현업에 가면 기존 코드를 리팩터링 하고, 기존 코드에 추가 기능을 구현하는 경험을 정말 많이 하게 될텐데,

지금 이 경험이 분명 현업에 가서 큰 도움이 될 거라 믿는다.

 

 

그리고 내 PR을 보면 알겠지만 리뷰어와 굉장히 많은 이야기를 주고 받았다.

내 코드 스타일과 리뷰어와 코드 스타일이 다른 경우가 많았다. (예외 처리, UiModel 등등..)

리뷰어는 어떤 이유에서 이러한 방식을 선호하는지, 이런 방식을 택했을 때의 장단점은 어떤 것이 있을지를 계속해서 궁금해 했다.

나와 다른 생각을 가진 사람과 대화를 나누면서 내 관점이 조금씩 더 넓어지는 것이 정말 즐거웠다.

리뷰어와 티키타카를 주고 받는 과정에서 가장 즐거움을 느낀 미션이었다.