#178 [refact] Rules 멀티 모듈로 Refactoring by murjune · Pull Request #179 · TeamHous/Hous-Android

murjune · 2022-08-11T21:18:50Z

작업 개요

Rules부분 멀티모듈로 리팩토링
Timber 적용

작업 설명

App -> HousApp으로 이름 바꿈

1) Timber 추가

Application클래스에서 Timber 설정
util패키지에 TimberDebugTree추가 후, Timber를 사용할 수 있도록 함

@HiltAndroidApp
class HousApp : Application() {
    override fun onCreate() {
        super.onCreate()
        if (BuildConfig.DEBUG) Timber.plant(TimberDebugTree())
    }
}

사용법

Timber.d("작업 내용") // 태그가 없는 것 빼고는 Log와 동일

참고블로그.

2) ApiModule

기존에는 NetWork모듈에 속해있는 Api주입하는 코드들을 Api모듈에 옮김

@Module
@InstallIn(SingletonComponent::class)
object ApiModule {
    @Provides
    @Singleton
    fun provideHomeService(retrofit: Retrofit): com.hous.data.api.HomeApi =
        retrofit.create(com.hous.data.api.HomeApi::class.java)
    ...
}

3) 멀티 모듈화.

이거는 커밋한 거를 봐주세연 > <

궁금한점

1. data모듈에도 timber dependency 추가했는게 괜찮나요??

ThirdPartyDependencies.run {
        implementation(interceptor)
        implementation(gson)
        implementation(timber) // Timber
        implementation(retrofit2)
        implementation(retrofit2Converter)
    }

1. 저는 repository가 아닌 dataSource에서 Mapping과 통신 성공여부를 처리해봤습니다. 괜춘한가여??
  이렇게 처리해준 이유는 만약 나중에 저희가 Room을 도입하게 된다면, Repository의 코드길이가 너무 늘어날 것 같아서입니당

class RemoteRulesTodayDataSourceImpl @Inject constructor(
    private val rulesApi: RulesApi
) :
    RemoteRulesTodayDataSource {
    private val ROOM_ID = BuildConfig.ROOM_ID

    override suspend fun getTodayTodayInfoList(roomId: String): RulesTodayInfo? =
        runCatching {
            rulesApi.getTodayTodayInfoList(ROOM_ID)
        }.fold(
            { response ->
                return RulesTodayInfo(
                    response.data?.homeRuleCategories!!.map { it.toCategoryInfo() },
                    response.data.todayTodoRules.map { it.toRuleInfo() })
            },
            {
                Timber.e(it.message)
                return null
            }
        )
        // 생략
}

UseCase는 이따 올릴게여 넘 졸려서..

KWY0218

전체적으로 아주 잘했네용 😄 😄 😄
궁금한 점

좋습니다
이건 에러 처리 관련 뷰나 얘기가 나오면 그 때 얘기 해야 될 것 같습니다.
그래야 room의 사용 여부가 결정될 것 같습니다

KWY0218 · 2022-08-12T00:21:07Z

domain/src/main/java/com/hous/domain/repository/RulesTodayRepository.kt

+        tmpRuleMembers: List<String>
+    ): Any?
+


viewModel 보니까 반환 값이 딱히 필요 없는 로직 같아 보여서
Any 말고 Unit 또는 반환 값을 없애도록 바꾸는 것이 좋을 것 같습니다.
자바로 따지면
Any -> Object
Unit -> Void
이런 거라고 알고 있습니다

근데 다시 생각해보니 그냥 안 넣는게 나은 것 같은 기분,,,?

정말 잘아시는군요!! 근데 Unit은 void+Void이긴합니당 ㅎ ㅅ ㅎ 여기서는 상관은없지만 ㅋㅋㅋ
관련 블로그

저는 RemonteDataSource에서 putTempManagerInfoList에서 서버 통신에 실패했을 때 null을 보내주어서 null값을 반환 안 할 때만 fetchToTodayList()함수를 호출하도록 로직을 구현했습니다. 하지만, Unit은 Non-Null타입이기 때문에 Any?을 반환타입으로 잡았습니다.

다른 put하는 함수에서는 모두 반환값을 없앴습니당!! 이 경우만 특수한 경우라 Any?타입으로 잡았네용

RemoteDataSourceIml

override suspend fun putTempManagerInfoList( roomId: String, ruleId: String, tmpRuleMembers: TempManagerRequest ): Any? { return runCatching { rulesApi.putTempManagerInfoList( ROOM_ID, ruleId, tmpRuleMembers ) }.fold( {}, { Timber.e(it.message) null // failure일 때 null 반환 } ) }

Repository

override suspend fun putTempManagerInfoList( roomId: String, ruleId: String, tmpRuleMembers: List<String> ): Any? { return remoteRulesTodayDataSource.putTempManagerInfoList( roomId, ruleId, TempManagerRequest(tmpRuleMembers) ) }

viewModel

viewModelScope.launch { rulesTodayRepository.putTempManagerInfoList( "", _todayTodoList.value!![tmpTodayToDoPosition.value!!].id, clickedTmpManagerList )?.let { fetchToTodayToDoList() } // null이 아닐 떄만 fetchToTodayToDoList() 호출

@murjune
아 통신 실패 시 null 값을 줘서 viewModel에서 null 일 때 에러 처리를 할 수 있겠군요~~
확인입니다~~!
null 값 처리 로직 timber라도 추가해주잉~~

@KWY0218 네넹!! 와 진짜 피드백 엄청 빠르네 이게 킹원용?! 저도 열심히 코드리뷰할게요~!~!
@KWY0218이거 배워갑니다 👍

KWY0218 · 2022-08-12T00:21:49Z

domain/src/main/java/com/hous/domain/model/Manager.kt

+        DayDataInfo("목", State.UNSELECT),
+        DayDataInfo("금ø", State.UNSELECT),
+        DayDataInfo("토", State.UNSELECT),


이거 금요일 옆에 저건 뭔가요,,,?

엇?? 이게 왜들어갔지

KWY0218 · 2022-08-12T00:24:01Z

data/src/main/java/com/hous/data/source/remote/RemoteRulesTodayDataSourceImpl.kt

+                return RulesTableInfo(
+                    response.data?.keyRules!!.map { it.toRuleInfo() },
+                    response.data.rules.map { it.toRuleInfo() }
+                )


Suggested change

return RulesTableInfo(

response.data?.keyRules!!.map { it.toRuleInfo() },

response.data.rules.map { it.toRuleInfo() }

)

return RulesTableInfo(

response.data?.keyRules!!.map { keyRulesList -> keyRulesList.toRuleInfo() },

response.data.rules.map { rulesList -> rulesList.toRuleInfo() }

)

우리 가독성을 위해 it 사용을 지양해 봅시다..!

네 알겠습니다!! 가독성 진짜 중요하죵 ㅎ ㅎ ㅎ

KWY0218 · 2022-08-12T00:31:02Z

app/src/main/java/com/hous/hous_aos/ui/rules/RulesViewModel.kt

+            val rulesTodayInfo: RulesTodayInfo? = rulesTodayRepository.getTodayTodayInfoList("")
+            rulesTodayInfo?.let {
+                Timber.d("RulesViewModel init")


rulesTodayInfo가 null일 때 timber를 띄운다는 로직을 추가하면 좋을 것 같습니다.
아직은 에러 처리에 대한 로직이 없지만 추후에 추가한다면
timber 자리에 에러 로직만 추가하면 되기 때문에 와드 같은 역할을 할 것 같습니다

오호라~!~! 알겠씁니당

murjune · 2022-08-16T20:32:23Z

널처리하다 현타와서.. 아직 다 하진 않았습니다..

확장함수 추가

/**
 * 변수 2가지 이상 null 체크할 때 사용 */
inline fun <T1 : Any, T2 : Any, R : Any> safeLet(p1: T1?, p2: T2?, block: (T1, T2) -> R?): R? {
    return if (p1 != null && p2 != null) block(p1, p2) else null
}

예시

todayTodoList.value?.let { todayTodoList ->
                tmpTodayToDoPosition.value?.let { tmpTodayToDoPosition ->
                    val id = todayTodoList[tmpTodayToDoPosition].id
                    rulesTodayRepository.putTempManagerInfoList(
                        "",
                        id,
                        clickedTmpManagerList
                    )

위의 코드를 다음과 같이 바꿀 수 있다!

safeLet(todayTodoList.value, tmpTodayToDoPosition.value) { 
todayTodoList: List<RuleInfo>, position: Int ->
                rulesTodayRepository.putTempManagerInfoList(
                    "",
                    todayTodoList[position].id,
                    clickedTmpManagerList
                )
            }

KWY0218 · 2022-08-17T00:59:33Z

@murjune
와우 확장함수 좋네용~~ LGTM
LiveData가 nullable이라서 값이 무조건 들어가도 null checking을 해야 돼서 생긴 함수군요.

바이럴 한번 하자면 stateFlow는 초기값이 무조건 들어가야 돼서 요런 일이 안 생긴답니다.
나중에 한번 잡솨봐유~~

마지막으로 릴리즈 레포에서는 확장 함수 위에 전에 올려준 kdocs 양식 맞춰서 주석을 달던가
github wiki를 사용하는 것이 좋을 것 같습니다.
이유는 해당 확장함수에 대해서 모르면 이 PR을 매번 찾아야 하는 불편함이 생길 것 같아서 입니다.

근데 생각해 보니까 릴리즈 레포가 private이라서 wiki는 못 쓸 거 같네요...
그 때 단톡방에 올려준 양식 지켜서 주석 달기 or 노션 안드 페이지에 따로 wiki 페이지를 하나 파도 괜찮을 것 같네용
다음 안드 회의 생길 때 논의해봅시다~

작업 하시느라 수고하셨습니다~ 👍

murjune · 2022-08-17T07:45:58Z

맞다!! kdocs양식 까먹었네유 ㅜㅜ
StateFlow 혼내줘보겠습니다 ㅋㅋㅋㅋ

murjune added 6 commits August 12, 2022 02:00

#178 [feat] ApiModule 추가

abf40f9

#178 [feat] App -> HousApp 네이밍 변경 및 Timber 적용

c6a7409

#178 [refact] domain에 model 이름 수정 및 추가

9c09cf1

#178 [refactor] domain 모듈로 repository 이동 및 domain model로 리팩토링

a8e0578

#178 [refact] data모듈 내 리팩토링

eebc8a9

#178 [refact] presentation 모듈 내 리팩토링

6402b5b

murjune added 🐻준원🐻 refact pull request🔥 labels Aug 11, 2022

murjune requested review from 2zerozu and KWY0218 August 11, 2022 21:18

murjune self-assigned this Aug 11, 2022

#178 [chore] klint 적용

f9d17dc

KWY0218 reviewed Aug 12, 2022

View reviewed changes

murjune added 4 commits August 17, 2022 04:52

#178 [fix] requireActivty() → viewLifecycleOwner 변경

0884bf6

#178 [fix] it -> 명시적인 변수 사용

e0d59a0

#178 [feat] safeLet 확장함수 추가

8ee6623

#178 [feat] null처리 로직 수정

9cbab11

KWY0218 self-requested a review August 17, 2022 01:52

murjune mentioned this pull request Sep 2, 2022

#7 [feat] 우리 집 규칙 기능 구현 Hous-Release/hous-AOS#8

Merged

Conversation

murjune commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

작업 개요

작업 설명

1) Timber 추가

2) ApiModule

3) 멀티 모듈화.

궁금한점

Uh oh!

KWY0218 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KWY0218 Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murjune Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KWY0218 Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

murjune Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KWY0218 Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

murjune Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

KWY0218 Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

murjune Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

KWY0218 Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

murjune Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

murjune commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

확장함수 추가

예시

Uh oh!

KWY0218 commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

murjune commented Aug 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

murjune commented Aug 11, 2022 •

edited

Loading

KWY0218 left a comment •

edited

Loading

KWY0218 Aug 12, 2022 •

edited

Loading

murjune Aug 12, 2022 •

edited

Loading

murjune Aug 12, 2022 •

edited

Loading

murjune commented Aug 16, 2022 •

edited

Loading

KWY0218 commented Aug 17, 2022 •

edited

Loading