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

[PC-256] 이용약관 페이지 UI 구현 #25

Merged
merged 26 commits into from
Jan 7, 2025
Merged

Conversation

tgyuuAn
Copy link
Member

@tgyuuAn tgyuuAn commented Jan 5, 2025

1. ⭐️ 변경된 내용

  • 이용 약관을 서버에서 동적으로 받아온 데이터를 보여줍니다.
  • 약관을 불러오는 로딩 시간을 없애기 위해 Splash에서 서버의 약관을 호출 후 로컬에 저장하고, 실제로는 로컬DB의 데이터를 호출하여 보여줍니다.
  • BaseResponse를 제네릭을 이용하여 더 간결하게 처리할 수 있도록 만들었습니다.
  • Check, CheckList 컴포넌트를 추가하였습니다.

2. 🖼️ 스크린샷(선택)

image image




image



3. 💡 알게된 부분

  • RetrofitApi 를 사용하는 부분에서 ApiResponse를 벗겨보려고 했지만..

뜻대로 되지 않았어요... Yapp 25기 안드로이드 단톡방을 보셔서 아시겠지만.. 여러명의 집단지성을 동원했는데도 실패했습니다 ㅠㅠ..




  • RoomTransactionTypeConverter을 처음으로 써봤어요!



4. 📌 이 부분은 꼭 봐주세요!

하다보니 코드가 너무 많아져서 죄송해요... 중간에 한 번 끊었어야 했는데 ㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠㅠ

작업을 더 하려고 했는데 일단 너무너무너무 길어져서 한 번 끊고 갑니다!



  • 테스트 추가해야할 부분이나 이름 바꿔야 할 부분 있는지 봐주시면 좋겠습니다!

@tgyuuAn tgyuuAn added UI/UX 🎨 디자인 시스템, 디자인 리소스 관련 코드 🎨 기능 ⚒️ 새로운 기능 구현 ⚒️ 새롭게 알게 됨 📖 프로젝트를 진행하며 새롭게 알게된 부분 📖 테스트 📜 내가 작성한 코드는 내가 책임진다 📜 ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn labels Jan 5, 2025
@tgyuuAn tgyuuAn self-assigned this Jan 5, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

제가 ProxyMan이라는 Tool을 사용해서 추가된 코드 입니다~~

Comment on lines +6 to +18
/**
* String?을 LocalDateTime으로 변환합니다.
*
* - 문자열이 null인 경우, [LocalDateTime.MIN]을 반환합니다.
* - 잘못된 형식으로 인해 파싱에 실패할 경우, [LocalDateTime.MIN]을 반환합니다.
*/
fun String?.parseDateTime(): LocalDateTime {
return try {
this?.let { LocalDateTime.parse(it) } ?: LocalDateTime.MIN
} catch (e: DateTimeParseException) {
LocalDateTime.MIN
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

일단 DefaultValue로 LocalDateTime.MIN을 반환하기로 하였는데,

에러를 바로 던져버릴 지 고민이에요...!

@tgyuuAn tgyuuAn requested a review from sksowk156 January 5, 2025 19:22
@tgyuuAn tgyuuAn marked this pull request as ready for review January 6, 2025 09:21
@tgyuuAn tgyuuAn added the 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 label Jan 6, 2025
Copy link
Contributor

@sksowk156 sksowk156 left a comment

Choose a reason for hiding this comment

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

허허 코멘트가 늦었습니다... 허허 정말 고생 많으셨습니다... 저도 빨리 작업해서 pr 올릴게요!!!

Comment on lines +12 to +18
fun String?.parseDateTime(): LocalDateTime {
return try {
this?.let { LocalDateTime.parse(it) } ?: LocalDateTime.MIN
} catch (e: DateTimeParseException) {
LocalDateTime.MIN
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p4 : try-catch도 좋지만, 코틀린의 runcatching을 많이 활용해보면 좋을 것 같은데 어떻게 생각하시나요?
fun String?.parseDateTime(): LocalDateTime {
    return this?.let {
        runCatching { LocalDateTime.parse(it) }
            .getOrDefault(LocalDateTime.MIN)
    } ?: LocalDateTime.MIN
}

Copy link
Contributor

Choose a reason for hiding this comment

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

  • p4 : String과 같은 범용 클래스를 global하게 확장 함수로 구현하는 대신, 도메인 모델인 Term 데이터 클래스 내부에 private한 확장 함수로 만들어 데이터 변환 시 사용하는 방법에 대해 어떻게 생각하시나요?

Copy link
Contributor

Choose a reason for hiding this comment

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

LocalDateTime.MIN 은 뭐가 나오나요?! 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p4 : try-catch도 좋지만, 코틀린의 runcatching을 많이 활용해보면 좋을 것 같은데 어떻게 생각하시나요?
fun String?.parseDateTime(): LocalDateTime {
    return this?.let {
        runCatching { LocalDateTime.parse(it) }
            .getOrDefault(LocalDateTime.MIN)
    } ?: LocalDateTime.MIN
}

try-catch나 runCatching으로 묶는 것 모두 에러를 잡기위한 코드인데,

Result객체를 사용하는 이유는 에러를 잡고 �이 책임을 다른 클래스에 위임하기 위해서 사용하는 것이라고 생각해요.

하지만, 위 코드는 해당 함수에서 에러를 잡고 바로 에러를 핸들링하기 때문에 오히려 코드 수도 늘어나고 불필요한 것 같습니다.

runCatching을 사용하면 try-catch보다 직관적이지 않고 코드수도 늘어난다고 생각해요.

어떻게 생각하시나요?!

관련 레퍼런스 하나 드릴게요! 에러 핸들링을 다른 클래스에게 위임하기

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p4 : String과 같은 범용 클래스를 global하게 확장 함수로 구현하는 대신, 도메인 모델인 Term 데이터 클래스 내부에 private한 확장 함수로 만들어 데이터 변환 시 사용하는 방법에 대해 어떻게 생각하시나요?

처음 이렇게 설계를 했지만,

추후에 Room을 사용하는 Database 모듈에서 해당 로직이 가짜 중복이 아닌 정말 진짜 중복으로 재사용 됨을 발견하였습니다.

Network 모듈에서는 ResponseDTO -> Domain Model로 변환,

Database 모듈에서는 RoomEntity -> Domain Model로 변환




그래서 진짜 중복이 발생한 두 코드를 위해 각 모듈에 각각 배치하는 것 보다 어차피 추후에 만들어진 common 모듈을 만들고 거기다가 넣는 것이 좋겠다고 판단하였습니다!

어떤가요?!

Copy link
Member Author

@tgyuuAn tgyuuAn Jan 7, 2025

Choose a reason for hiding this comment

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

LocalDateTime.MIN 은 뭐가 나오나요?! 🤔

아래�사진과 같은 시간이 나와요!

image

이는 ResponsDTO -> Domain Model로 파싱할 때 처럼 파싱에 실패했을 때 UNKNOWN 과 같은 효과를 주는 것 처럼 설계하였어요!

이는 Repository에서 DomainModel로 매핑할 때 필터링 되어야합니다!!

Copy link
Contributor

Choose a reason for hiding this comment

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

  • 다시 보니 runcatching 보단 try-catch가 가독성이 좋아보이는 것 같아요! 👍
  • 아하 그렇군요!! 그럼 질문이 하나 있습니다! 그럼 common은 network, database 모듈에서만 접근할 수 있는 건가요?? 아니면 다른 모듈들에서 접근할 수 있게 되는건가요??
  • 허허 그렇군요!! 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

  • 아하 그렇군요!! 그럼 질문이 하나 있습니다! 그럼 common은 network, database 모듈에서만 접근할 수 있는 건가요?? 아니면 다른 모듈들에서 접근할 수 있게 되는건가요??

common 모듈은 프레임워크 의존성이 없는 모듈이라서 어떤 모듈이라도 해당 모듈에 접근 가능하도록 설계하였습니다!

import org.junit.jupiter.api.Test
import java.time.LocalDateTime

class TimeUtilTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

허허 테스트 코드의 시작을...!!!! 👍 👍

Comment on lines +34 to +36
override suspend fun getTerms(): Result<List<Term>> = runCatching {
localTermDataSource.getTerms()
.map { it.toDomain() }
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p3 : get 보다 더 스코프한 표현이면 좋을 것 같은데 어떻게 생각하시나요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p3 : get 보다 더 스코프한 표현이면 좋을 것 같은데 어떻게 생각하시나요?!

헉, 스코프하다라는 것이 어떤 뜻인가요?!

좀 더 광범위 ...? 음.. 해당 함수는 약관을 불러오는 함수를 뜻하는데요,

혹시 더 직관적이고 좋은 뜻이 있을까요 ?!

Copy link
Contributor

Choose a reason for hiding this comment

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

앗 죄송합니다 표현이 명확하지 않았던 것 같아요! 제 말은 조금 더 좁은 범위, 의미를 가진 단어이면 좋을 것 같다는 뜻이었어요!!

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 개인적으로 local에서 데이터를 가져올 경우엔 retrieve라는 표현을 자주 사용하는 것 같아요!
옥스포드사전을 보시면 retrieve의 사전적 정의도 그렇고,
grep에서 해당 단어를 조회해 보면 google api 문서나 다른 여러 문서에서 retrieve를 뭔가 저장되어 있는 데이터를 불러올 때 쓰는 것 같아요! 뭐 비슷한 의미를 가진 단어들이 많아 네이밍에 어려움을 겪을 때는 주로 gpt, grep, 옥스포드 사전을 참고하는 편입니다....!!!!

Copy link
Member Author

Choose a reason for hiding this comment

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

오오 새로 배웁니다! 다음 PR에서 retrieve로 바꿔놓겠습니다!

val title: String,
val content: String,
val required: Boolean,
@ColumnInfo(name = "start_date") val startDate: LocalDateTime,
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 TermEntity의 startDate를 String이 아닌 LocalDateTime으로 하신 이유가 있으신가요?

Copy link
Member Author

@tgyuuAn tgyuuAn Jan 7, 2025

Choose a reason for hiding this comment

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

혹시 TermEntity의 startDate를 String이 아닌 LocalDateTime으로 하신 이유가 있으신가요?

DomainModel을 왜 LocalDateTime으로 하였느냐가 궁금하신건지,

아니면 TermEntity에 String이 아닌 LocalDateTime을 쓰신건지 궁금합니다!



DomainModel을 왜 LocalDateTime으로 하였느냐가 궁금하시다면,

DomainModel인 Term에서 LocalDateTime을 사용한 이유는 해당 프로퍼티를 기반으로 매칭 홈 UI에서 초 단위로 변하는 UI를 그려야하기 때문에 이 계산에 용이한 LocalDateTime으로 설계를 하였습니다.

(사실 서버측에서 LocalDateTime형식으로 내려주셔서 DomainModel에서는 저 타입을 쓰는 것이 자명해요)



TermEntity에 String이 아닌 LocalDateTime을 쓰신건지 궁금하시다면,

TermEntity에 LocalDateTime을 넣은 이유는 원래 DTO는 String으로 저장되는 것이 일반적이지만,

Room은 TypeConverter라는 것을 이용해서 데이터베이스에서 지원하지 않는 타입 _(예: LocalDateTime, Enum)_을 지원할 수 있게 해주는데,

데이터를 삽입할 때 변환하여 데이터베이스에 저장하고, 조회할 때 다시 변환하여 객체 형태로 사용할 수 있게 되어요!

Copy link
Contributor

Choose a reason for hiding this comment

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

제 질문은

TermEntity에 LocalDateTime을 넣은 이유는 원래 DTO는 String으로 저장되는 것이 일반적이지만,

Room은 TypeConverter라는 것을 이용해서 데이터베이스에서 지원하지 않는 타입 _(예: LocalDateTime, Enum)_을 지원할 수 있게 해주는데,

데이터를 삽입할 때 변환하여 데이터베이스에 저장하고, 조회할 때 다시 변환하여 객체 형태로 사용할 수 있게 되어요!

이 부분에 대한 질문이었습니다! 상세한 답변 감사합니다 ㅎㅎ

@Entity(tableName = "term")
data class TermEntity(
@PrimaryKey
@ColumnInfo(name = "term_id") val termId: Int,
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p4 : 그냥 term 없이 id여도 괜찮을 것 같은데 어떻게 생각하시나요!?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p4 : 그냥 term 없이 id여도 괜찮을 것 같은데 어떻게 생각하시나요!?

좋아요!!

val termsCheckedInfo: MutableMap<Int, Boolean> = mutableMapOf(),
) : MavericksState {
val agreeAllTerms = terms.all { termsCheckedInfo.getOrDefault(it.termId, false) }
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p3 : agreeAllTerms은 뭔가 동사의 느낌이 강한 것 같습니다! allTermsAgreed 나 allTermsAccepted, isAllTermsAgreed 등은 어떻게 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • p3 : agreeAllTerms은 뭔가 동사의 느낌이 강한 것 같습니다! allTermsAgreed 나 allTermsAccepted, isAllTermsAgreed 등은 어떻게 생각하시나요?

좋은 것 같습니다!!


private fun processIntent(intent: RegistrationIntent) {
when (intent) {
is RegistrationIntent.Navigate -> navigationHelper.navigate(intent.navigationEvent)
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p3 : 아마 저도 matching 쪽에 navigate를 intent로 던졌던 것 같은데... 🤔 뭔가 state를 변경하는 것이 아니라면 intent보단 side effect에서 처리하는게 맞지 않을가 생각하는데 어떻게 생각하시나욥?!!

Copy link
Member Author

@tgyuuAn tgyuuAn Jan 7, 2025

Choose a reason for hiding this comment

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

  • p3 : 아마 저도 matching 쪽에 navigate를 intent로 던졌던 것 같은데... 🤔 뭔가 state를 변경하는 것이 아니라면 intent보단 side effect에서 처리하는게 맞지 않을가 생각하는데 어떻게 생각하시나욥?!!

Navigation의 경우 sideEffect에서 처리하는 게 맞는 것 같아요!

SideEffect로 호출 -> SideEffect 에서 navigationHelper 호출 이 되는 거겠죠 ??

}
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

허허 꼼꼼한 테스트 작성 👍 👍

@Before
fun setUp() {
val context = ApplicationProvider.getApplicationContext<Context>()
db = Room.inMemoryDatabaseBuilder(
Copy link
Contributor

Choose a reason for hiding this comment

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

아하 room 테스트는 이렇게 하는군요!

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 room 테스트는 이렇게 하는군요!

Room은 테스트할 때 메모리 상에 데이터를 올려서 테스트가 끝나면 데이터베이스가 지워지는 InMemoryDatabase를 사용한답니다!!

label: String,
onCheckedChange: () -> Unit,
modifier: Modifier = Modifier,
showArrow: Boolean = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

  • p3 : showArrow는 뭔가 동사 느낌이 있어서 메서드처럼 느껴지는 것 같은데 혹시 어떻게 생각하시나요?

Copy link
Member Author

@tgyuuAn tgyuuAn Jan 7, 2025

Choose a reason for hiding this comment

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

  • p3 : showArrow는 뭔가 동사 느낌이 있어서 메서드처럼 느껴지는 것 같은데 혹시 어떻게 생각하시나요?

�arrowEnabled로 변경하였습니다!

@tgyuuAn tgyuuAn added 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟 and removed 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 labels Jan 7, 2025
@tgyuuAn
Copy link
Member Author

tgyuuAn commented Jan 7, 2025

KakaoTalk_Snapshot_20250107_135132

다음 PR에 계속 ..

@tgyuuAn tgyuuAn merged commit b6a187f into develop Jan 7, 2025
1 check passed
@tgyuuAn tgyuuAn deleted the feature/tgyuu/PC-256 branch January 7, 2025 04:51
Copy link
Contributor

@sksowk156 sksowk156 left a comment

Choose a reason for hiding this comment

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

허허 아주 좋은 PR이었던 것 같아요 많이 배웠습니다 태규님...!! 🙇

Comment on lines +34 to +36
override suspend fun getTerms(): Result<List<Term>> = runCatching {
localTermDataSource.getTerms()
.map { it.toDomain() }
Copy link
Contributor

Choose a reason for hiding this comment

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

앗 죄송합니다 표현이 명확하지 않았던 것 같아요! 제 말은 조금 더 좁은 범위, 의미를 가진 단어이면 좋을 것 같다는 뜻이었어요!!

Comment on lines +12 to +18
fun String?.parseDateTime(): LocalDateTime {
return try {
this?.let { LocalDateTime.parse(it) } ?: LocalDateTime.MIN
} catch (e: DateTimeParseException) {
LocalDateTime.MIN
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 다시 보니 runcatching 보단 try-catch가 가독성이 좋아보이는 것 같아요! 👍
  • 아하 그렇군요!! 그럼 질문이 하나 있습니다! 그럼 common은 network, database 모듈에서만 접근할 수 있는 건가요?? 아니면 다른 모듈들에서 접근할 수 있게 되는건가요??
  • 허허 그렇군요!! 👍

val title: String,
val content: String,
val required: Boolean,
@ColumnInfo(name = "start_date") val startDate: LocalDateTime,
Copy link
Contributor

Choose a reason for hiding this comment

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

제 질문은

TermEntity에 LocalDateTime을 넣은 이유는 원래 DTO는 String으로 저장되는 것이 일반적이지만,

Room은 TypeConverter라는 것을 이용해서 데이터베이스에서 지원하지 않는 타입 _(예: LocalDateTime, Enum)_을 지원할 수 있게 해주는데,

데이터를 삽입할 때 변환하여 데이터베이스에 저장하고, 조회할 때 다시 변환하여 객체 형태로 사용할 수 있게 되어요!

이 부분에 대한 질문이었습니다! 상세한 답변 감사합니다 ㅎㅎ

Comment on lines +9 to +10
val status: String?,
val message: String?,
Copy link
Contributor

Choose a reason for hiding this comment

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

DataSource 클래스에서 밑에 만들어주신 unwrapData 확장함수로 모든 api 데이터들에서 data를 뽑아서 보내주는데 그 과정에서 message와 status는 사용되지 않는 것 같은데 언제 사용할 수 있는지 궁금합니다! 만약에 사용하지 않는 다면 없어도 되지 않나 싶은데 혹시 어떻게 생각하시나요?

Comment on lines +18 to +24
data class TermResponse(
val termId: Int?,
val title: String?,
val content: String?,
val required: Boolean?,
val startDate: String?,
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

생각해보니 태규님 말씀대로 toDomain에서 처리하는 추후에 다른 converter를 적용했을 때 수정하기 좋은 방식인 것 같아요!! 👍

Comment on lines +44 to +46
checkAllTerms: () -> Unit,
checkTerm: (Int) -> Unit,
navigate: (NavigationEvent) -> Unit,
Copy link
Contributor

Choose a reason for hiding this comment

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

허허 뭔가 제 의도와는 다른 아주 고퀄리티의 답변을 해주신 것 같습니다...!! 👍 👍

태규님이 답변 주신 내용과 비슷하면서 조금 다른 고민을 한 적이 있는데, 예전에 MVI 패턴을 공부하면서 많이 고민했던 부분인데, 제가 onCheckedChange 와 같이 클릭 이벤트명과 비슷하게 짓게된 이유는 onCheckedChange 가 뭔가 사용자 행동에만 초점을 맞추는 느낌이라고 생각했어요. ui는 state가 어떻게 변경되었는지만 알면 되고(어떤 기능을 하는 버튼을 눌렀는지), 그것의 의도는 intent를 받는 viewmodel에서만 알고 처리하면 된다고 생각했어요. 그럼 자연스럽게 태규님이 말씀해주신 것처럼 각 클릭 이벤트를 람다로 내려줘야 하는 일이 발생하게 되는 것 같아요.

그런데 사실 전 이런 의도로 말씀드린 건 아니였어요...!!! ㅋㅋㅋ 그런데 이런 아주 고퀄리티의 답변을 받으니 정말 태규님은 진짜구나 라는 생각이 많이 드네요...!! ㅋㅋㅋ
태규님이 설명해주신 글을 보고 생각해보니까 태규님의 방식이 더 가독성 있고 작업 시간이 줄어들 것 같다는 생각이 드네요...!! 👍

Comment on lines +34 to +36
override suspend fun getTerms(): Result<List<Term>> = runCatching {
localTermDataSource.getTerms()
.map { it.toDomain() }
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 개인적으로 local에서 데이터를 가져올 경우엔 retrieve라는 표현을 자주 사용하는 것 같아요!
옥스포드사전을 보시면 retrieve의 사전적 정의도 그렇고,
grep에서 해당 단어를 조회해 보면 google api 문서나 다른 여러 문서에서 retrieve를 뭔가 저장되어 있는 데이터를 불러올 때 쓰는 것 같아요! 뭐 비슷한 의미를 가진 단어들이 많아 네이밍에 어려움을 겪을 때는 주로 gpt, grep, 옥스포드 사전을 참고하는 편입니다....!!!!

private val termsDao: TermsDao,
) {
suspend fun getTerms() = termsDao.getTerms()
suspend fun clearAndInsertTerms(terms: List<TermEntity>) =
Copy link
Contributor

Choose a reason for hiding this comment

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

아하 그렇군요 저는 보통 그런 상황에서 replace라는 표현을 쓰긴합니다! 근데 사실 지금도 네이밍이 이해하기 쉽고 명확해서 굳이 바꿀 필요는 없을 것 같습니다!!

val terms: List<Term> = emptyList(),
val termsCheckedInfo: MutableMap<Int, Boolean> = mutableMapOf(),
Copy link
Contributor

Choose a reason for hiding this comment

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

저희는 RegistrationState를 data class로 사용하면서 copy를 통해 state를 갱신하기 때문에 데이터의 불변성을 어느 정도 지키고 있다고 생각해요. 하지만, MutableMap을 사용할 경우 copy를 하기 전 비즈니스 로직을 처리하는 과정에서 불변성이 의도치 않게 깨질 가능성이 있어요...!!(실제로는 거의 발생하지 않을 것이라 생각합니다). 이는 val 키워드가 참조 자체를 변경하지 못하게 하지만, 내부 객체의 변경은 막지 못하기 때문이죠. 관련해서 블로그가 잘 정리되어 있는 것 같아요! 그래서 map을 쓰는게 더 좋을 것 같아요. 만약 변경할 일이 생기면 그때 toMutable...로 변경해서 써도 되니깐..!

그런데 아시다시피 map이나 list을 써도불변성이 완전히 보장되지는 않아요. 예를 들어 map 변수에 다른 값을 할당하거나 캐스팅하게 될 경우 변경할 수 있기 때문이죠..! 그래서 immutableList나 persistentList 혹은 일급 컬렉션 등으로 이러한 현상을 피하긴 하는데. 개인적으론 이 데이터 클래스가 ui State이기 때문에 일급 컬렉션을 만들어서 쓸 필요는 없다고 생각해요...!! 그리고 지금 정도의 비지니스 로직의 복잡도라면 그냥 list나 map 정도만으로 충분하지 않나 생각합니다...!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX 🎨 디자인 시스템, 디자인 리소스 관련 코드 🎨 ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn 기능 ⚒️ 새로운 기능 구현 ⚒️ 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟 새롭게 알게 됨 📖 프로젝트를 진행하며 새롭게 알게된 부분 📖 테스트 📜 내가 작성한 코드는 내가 책임진다 📜
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants