Skip to content

상세화면 ViewModel 리펙토링

양성현 edited this page Dec 14, 2022 · 4 revisions

약속 상세 화면은 팀 프로젝트 이따 만나의 메인 화면 중 하나이다.

약속 정보 표시부터 지도 기능, 마커 기능, 지도 카메라 이동 기능, 알림 기능, 위치 공유 기능 등 많은 일이 한 화면에서 일어나고 있다.

본인을 포함한 모든 팀원이 작업을 한 부분이고, 그러다 보니 각자 구현한 기능들이 약속 상세 화면에 추가되었고 현재 우리를 어지럽게 하는 코드가 되어있다.

그래서 기능 추가나 버그 수정과 같은 다른 우선순위가 높은 작업이 있었지만

먼저 해당 코드들이 먼저 개선되어야 한다고 생각했고, 그래서 상세 화면의 뷰 모델을 개선해보기로 했다.

1. 중복 제거하기

val promise: SharedFlow<Promise> = _promiseInfo.mapNotNull { it }.shareIn(viewModelScope, SharingStarted.Eagerly, 1)

private val _promiseInfo = MutableStateFlow<Promise?>(null)
val promiseInfo: StateFlow<Promise?> get() = _promiseInfo.asStateFlow()

뷰 모델에 똑같은 약속 정보를 가지는 멤버 변수들이 있다.

각자 다른 사람이 구현한 것으로 보이는데 둘의 공통점은 _promiseInfo를 사용하고 있다는 것이다.

다른 점은

  • promise 같은 경우 SharedFlow이고 Nullable이 아니다.
  • promiseInfo StateFlow에 들어있는 데이터가 Nullable이다.

불필요하게 구현되어있기 때문에 이 중복을 제거하기로 했다.

private val _promise = MutableStateFlow<Promise?>(null)
val promise: Flow<Promise> = _promise.filterNotNull()
  • nullable인 promiseInfo를 삭제했다. nullable이라서 promiseInfo를 사용하는 곳마다 null 체크를 하고 있기 때문이다.
  • MutableStateFlow인 _promisefilterNotnull()로 가져오는 것으로 수정했다. mapNotNull() 로 null을 제외하고 가져오는 것보다 filterNotNull()이 의미가 바르다고 생각했다. 또한 SharedFlow으로 사용할 필요가 없다고 생각해서 shareIn도 삭제했다.

2. AssistedInject를 통해 필요한 값을 넘겨주자

private val _promise = MutableStateFlow<Promise?>(null)
val promise: Flow<Promise> = _promise.filterNotNull()

fun setPromise(promiseId: String) {
    viewModelScope.launch {
        _promise.value = promiseRepository.getPromise(promiseId).first().copy()
        promise.collectLatest {promise->
            memberMarkers = List(promise.members.size) { Marker() }
            isAcceptLocationSharing.value = memberRepository.getIsAcceptLocation(promiseId).first().getOrElse{false}
        }
    }
}

현재 뷰 모델의 _promise는 뷰 모델을 사용하는 activity에서 setPromise 함수를 호출해야 _promise에 값이 들어온다.

이렇게 함수를 실행해야 StateFlow가 업데이트되는 형태의

장점은

  • 원하는 타이밍에 업데이트를 실행할 수 있다.

단점은

  • 함수가 호출되기 전까지 업데이트가 되지 않는다.
  • 함수를 여러 번 호출할 때 여러 job이 생기게 된다.

setPromise 함수는 매개변수로 들어온 promiseId로 약속을 서버에서 가져와서 _promise에 값을 넣는 것뿐만 아니라 promisecollectLatest해서 memberMarkers를 초기화하거나 isAcceptLocationSharing을 업데이트하는 일까지 하고 있다.

promise는 PromiseDetailViewModel에서 반드시 있어야 할 데이터이다.

그래서 함수를 통해서 약속을 업데이트하는 것이 아니라 뷰 모델 생성자로 promiseId를 받는 것으로 수정하기로 했다.

현재 우리 프로젝트는 Hilt를 사용하고 있고 그래서 AssistedInject를 사용하기로 했다.

class PromiseDetailViewModel @AssistedInject constructor(
    ...
    @Assisted promiseId: String
) : ViewModel() {
    private val _promise: StateFlow<Promise?> = promiseRepository.getPromise(promiseId)
        .stateIn(viewModelScope, SharingStarted.Eagerly, null)

		val promise: Flow<Promise> = _promise.filterNotNull()

promiseId를 생성자로 받아올 수 있게 되었고

setPromise함수를 사용할 필요 없게 되었다.

3. 덩어리 코드 분리하기

@OptIn(ExperimentalCoroutinesApi::class)
val memberLocations: Flow<List<MemberUiModel>?> = promiseInfo.flatMapLatest { promise->
    if (promise != null) {
        val users = promise.members

        memberRepository.getMembers(promise.promiseId).flatMapLatest {members->
            val geoLocations = locationRepository.getGeoLocations(
                members.map{member-> member.userCode}
            )
            geoLocations.map {userGeoLocations->
                userGeoLocations.map {userGeoLocation->
                    MemberUiModel(
                        userGeoLocation.userCode,
                        users.find{user-> user.userCode == userGeoLocation.userCode}?.userName ?: "",
                        userGeoLocation.geoLocation
                    )
                }
            }
        }
    } else {
        flow {emit(null)}
    }
}

상세 화면 뷰 모델에는 많은 블록 들여쓰기를 한 코드가 있었다.

MemberUiModel은 약속 멤버의 code, name, 도착 여부를 가진 데이터로 보인다. MemberUser, UserGeoLocation 값들이 필요하고 이를 조합해 MemberUiModel을 만들고 있다.

  • 여기 말고도 멤버 geoLocations를 사용하는 곳이 있다. 그래서 중복으로 서버에 멤버 geoLocations을 요청하고 있다.
  • 코드가 분리되지 않아서 재사용하거나 수정, 테스트가 쉽지가 않다.

그래서 memebersuserGeoLocations 그리고 memberUiModels로 나누기로 했다.

처음 코드를 아래와 같이 분리하게 되었다.

@OptIn(ExperimentalCoroutinesApi::class)
private val members: Flow<List<Member>> = promise.flatMapLatest{ promise->
    val myInfoUserCode = myInfo.first().getOrElse { throw IllegalStateException() }.userCode
    memberRepository.getMembers(promise.promiseId).map { members->
        members.filter { it.userCode != myInfoUserCode }
    }
}
@OptIn(ExperimentalCoroutinesApi::class)
val userGeoLocations: Flow<List<UserGeoLocation>> = members.flatMapLatest { members->
    val locationSharingAcceptMembers = members.filter { member->
        member.isAcceptLocation
    }.map { member->
        member.userCode
    }
    if (locationSharingAcceptMembers.isNotEmpty()) {
        locationRepository.getGeoLocations(locationSharingAcceptMembers)
    } else {
       flow{emit(emptyList())}
    }
}
val memberUiModels: Flow<List<MemberUiModel>?> = userGeoLocations.map { userGeoLocations->
    val myUserCode = myInfo.first().getOrElse { return@map null }.userCode
    promise.first().members.filter {
        it.userCode != myUserCode
    }.map { user->
    val userGeoLocation = userGeoLocations.find { it.userCode == user.userCode }
    val destination = promise.first().destinationGeoLocation
        MemberUiModel(
            userCode = user.userCode,
            userName = user.userName,
            isArrived = isArrival(destination, userGeoLocation)
        )
    }
}

members, userGeoLocations, memberUiModels 3개로 분리했다.

하나로 뭉쳐있던것을 memebersuserGeoLocations 그리고 memberUiModels로 나누어진 덕분에 읽기 한결 쉬워졌고 수정, 사용, 테스트에 유용하게 되었다.

4. 테스트 코드 작성하기

상세: Unit Test를 해보자
기존에 제대로 수정되었는지 확인하기 위해서는 코드마다 Log를 달아주고, 안드로이드 에뮬레이터를 실행해서 직접 클릭해봐야 했다.

버그 fix, 기능추가, 리팩토링할 때마다 테스트가 필요하므로 테스트 코드를 작성해보았다.

빠르게 뷰 모델의 로직만 검증하기 위해 유닛테스트로 테스트 코드를 작성했다.

뷰 모델은 UserRepository, PromiseRepository 등 데이터를 받아오기 위한 값들이 있어야 하는데 Mockito를 통해 해결할 수 있었다.

Mock을 사용하면

  • 유닛 테스트를 하면서 DataStore나 Room처럼 안드로이드 종속 항목이 있을때 Mock 객체로 대체할 수 있다.
  • 테스트하려는 부분에 집중할 수 있다.
  • 빠르게 테스트할 수 있다.
@RunWith(MockitoJUnitRunner::class)
class PromiseDetailViewModelTest {

    @Mock
    private lateinit var promiseRepository: PromiseRepository

    @Mock
    private lateinit var userRepository: UserRepository
    ...
@OptIn(ExperimentalCoroutinesApi::class)
@Test
fun getPromise_Success() = runTest {
    `when`(promiseRepository.getPromise(fakePromise.promiseId)).thenReturn (
        flow {
            emit(fakePromise)
        }
    )
    `when`(userRepository.getMyInfo()).thenReturn(
        flow {
            emit(Result.success(myInfo))
        }
    )
    val promiseDetailViewModel = PromiseDetailViewModel(
        promiseRepository,
        userRepository,
        memberRepository,
        locationRepository,
        notificationRepository,
        alarmDirector,
        promiseId
    )

    val result = promiseDetailViewModel.promise.first()

    assertEquals(fakePromise, result)
}

본래 Android 종속성을 요구하지만 Mockito를 통해 Mock 객체를 만들어서 빠르게 뷰 모델의 로직을 검증할 수 있었다.

완벽한 코드라는 게 없고 실수를 할 수 있으므로 작성한 로직이 의도된 대로 돌아가는지 확인하기 위한 테스트 코드 작성의 필요성이 느껴진다.

references

Dagger Hilt Assisted Inject- 런타임 주입하기

레거시 코드(Legacy code)

로컬 단위 테스트 빌드 | Android 개발자 | Android Developers

Android Mockito로 Unit 테스트 코드 작성하기 (kotlin)

Clone this wiki locally