refactor: BindingAdapter 분리 및 리팩토링 #469 #472

s6m1n · 2024-10-10T13:10:18Z

⭐️ Issue Number

refactor: BindingAdapter 분리 및 리팩토링 #469

🚩 Summary

View 별 파일 분리

ButtonBindingAdapters : Button, MaterialButton, ImageButton의 확장함수
ImageBindingAdapters : ImageView의 확장함수
TextBindingAdapters : TextView의 확장함수
BindingAdapters : View ViewGroup 등 기타 뷰들의 확장함수

추억 수정 화면 DatePicker 버그 수정

isAdded를 검사해 dateRangePicker의 show가 중복 호출되며 앱이 crash되는 버그를 수정했습니다.

    override fun onPeriodSelectionClicked() {
        if (dateRangePicker.isAdded) {
            dateRangePicker.show(supportFragmentManager, dateRangePicker.toString())
        }
    }

hxeyexn

빙티!! BIndingAdpater 분리하느나 고생많으셨습니다ㅠㅠ
BIndingAdpater 찾기 너무 힘들었는데 빙티 덕분에 이제 쉽게 찾을 수 있겠네용 헤헤 👍😘

hxeyexn · 2024-10-14T05:23:13Z

.../Staccato_AN/app/src/main/java/com/on/staccato/presentation/moment/feeling/FeelingUiModel.kt

+    @ColorRes val colorSrc: Int?,
+    @ColorRes val graySrc: Int?,


@ColorRes 명시 좋네요!! 👍

hxeyexn · 2024-10-14T05:23:48Z

android/Staccato_AN/app/src/main/java/com/on/staccato/presentation/common/LocalDateFormatter.kt

+        val hour = if (setNowDateTime.hour % 12 == 0) 12 else setNowDateTime.hour % 12
+        val noonText = if (setNowDateTime.hour < 12) "오전" else "오후"


하드코딩된 값들은 상수화하는 건 어떨까요?

12는 상수로, 오전 & 오후는 strings.xml로 분리했습니다!

hxeyexn · 2024-10-14T05:27:56Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/EtxBindingAdapters.kt

파일 이름에 오타가 있는 것 같아요..!! Etc~

뷰 종류에 상관 없는 메서드들을 담은 파일은 BindingAdapters.kt로 수정했습니다!

hxeyexn · 2024-10-14T05:40:59Z

...ato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/ButtonBindingAdapter.kt

+@BindingAdapter("recoveryButtonEnabled")
+fun Button.setRecoveryButtonEnabled(recoveryCode: String?) {
+    isEnabled =
+        if (recoveryCode.isNullOrBlank() || recoveryCode.length < 36) {


36도 상수화하면 좋을 것 같아요~

hxeyexn · 2024-10-14T05:42:57Z

...cato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/ImageBindingAdapter.kt

+@BindingAdapter(
+    value = ["coilImageUri", "coilPlaceHolder"],
+)
+fun ImageView.loadCoilImageByUri(
+    uri: Uri?,
+    placeHolder: Drawable? = null,
+) {
+    load(uri) {
+        placeholder(placeHolder)
+        error(placeHolder)
+    }
+}


해당 BindingAdapter는 사용되고 있지 않는데 제거해도 되지 않을까요?

제거했습니다!

hxeyexn · 2024-10-14T05:50:31Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/TextBindingAdapter.kt

+
+@BindingAdapter("photoDragHintVisibility")
+fun TextView.setPhotoDragHintVisibility(currentPhotoNumbers: Int) {
+    isGone = currentPhotoNumbers < 2


여기도 상수화하면 좋을 것 같아요~

hxeyexn · 2024-10-14T05:55:49Z

android/Staccato_AN/app/src/main/res/layout/activity_memory_creation.xml

+                    android:visibility="@{viewModel.isPeriodActive ? View.VISIBLE : View.GONE }"
                    app:layout_constraintEnd_toEndOf="parent"
                    app:layout_constraintStart_toStartOf="parent"
                    app:layout_constraintTop_toBottomOf="@id/tv_memory_creation_period_description"
                    bind:endDate="@{viewModel.endDate}"
-                    bind:periodSelectionVisibility="@{viewModel.isPeriodActive}"


EtcBindingAdapter에 있는 setVisibility 함수를 활용해도 될 것 같아요~

hxeyexn · 2024-10-14T06:17:16Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/TextBindingAdapter.kt

+fun TextView.formatVisitedAtHistory(visitedAt: LocalDateTime?) {
+    text = visitedAt?.let {
+        getFormattedLocalDateTime(it) + resources.getString(R.string.visit_at_history)
+    } ?: ""


""도 전반적으로 상수화하면 좋을 것 같아요!

hxeyexn · 2024-10-14T06:24:50Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/TextBindingAdapter.kt

+fun TextView.formatVisitedAtHistory(visitedAt: LocalDateTime?) {
+    text = visitedAt?.let {
+        getFormattedLocalDateTime(it) + resources.getString(R.string.visit_at_history)


Suggested change

fun TextView.formatVisitedAtHistory(visitedAt: LocalDateTime?) {

text = visitedAt?.let {

getFormattedLocalDateTime(it) + resources.getString(R.string.visit_at_history)

@BindingAdapter("visitedAtHistory")

fun TextView.formatVisitedAtHistory(visitedAt: LocalDateTime?) {

text = visitedAt?.let {

resources.getString(R.string.visit_at_history).format(getFormattedLocalDateTime(it))

// fragment_visit <string name="visit_at_history">%s에 방문했어요</string>

visit_at_history에 %s를 추가하고 format을 활용하는 건 어떠신가용?

반영했습니다!

Junyoung-WON

바인딩 어댑터 리팩터링 너무 고생하셨습니다! 💯
정말 깔끔하게 잘 나누어주신 것 같아요~~
이제 바인딩 어댑터를 찾으러 몇백줄을 뒤져보지 않아도 되겠군요....! 🥹
너무 좋습니다!! 😆

몇 가지 확인해보면 좋을 부분에 대해서 코멘트 남겨드렸으니, 확인 부탁드립니다!

Junyoung-WON · 2024-10-14T06:16:09Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/EtxBindingAdapters.kt

Etc 를 Etx 로 오타를 내신 것 같네요! 😂
분리를 하고 나머지 뷰들의 바인딩 어댑터임을 명시하기 위해 파일 이름을 이와 같이 변경하신 것 같은데,
저는 기존의 BindingAdapters.kt 가 좋아보여요!

기존에 존재하던 BindingAdapters 에서 너무 많은 메서드를 가진 View의 경우만 분리를 해주신 것이니,
"남은 자잘한 뷰들의 바인딩 어댑터에요~" 하고 파일 이름에 나타낼 이유는 크게 없어보였어요!

그리고 빙티가 파일 분리를 하면서 어떤 View의 바인딩 어댑터인지 파일 이름에 잘 명시해주셨어요.
그래서 이를 활용할 동료 개발자의 입장에서도 혼란이 일어날 것 같지 않아요~

반영했습니다!

Junyoung-WON · 2024-10-14T06:40:28Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/EtxBindingAdapters.kt

+@BindingAdapter("scrollableToBottom")
+fun ScrollView.isScrollableToBottom(isScrollable: Boolean) {


다른 화면의 스크롤 뷰에도 적용할 수 있도록 메서드와 파라미터 이름을 변경하신 것 같네요!

메서드의 파라미터로 들어온 Boolean 값이 true 인 경우, 스크롤을 맨 하단으로 강제 이동

해당 바인딩어댑터는 위와 같이 조건에 의해 수행되고 강제성을 띄기 때문에, 가능성을 나타내는 scrollable 이라는 이름이 들어간 것이 어색하게 느껴졌어요.
비슷한 예로 View의 가시성을 설정하는 바인딩어댑터의 메서드 명은 가능성을 나타내고있지 않죠!

@BindingAdapter("visibleOrGone") fun View.setVisibility(isVisible: Boolean?)

바인딩 어댑터 메서드 명의 통일성을 유지하고,
값이 true인 경우 맨 아래로 스크롤한다 라는 동작이 잘 나타나도록 아래와 같은 예시를 참고해보는 것은 어떨까요?

Suggested change

@BindingAdapter("scrollableToBottom")

fun ScrollView.isScrollableToBottom(isScrollable: Boolean) {

@BindingAdapter("scrollToBottom")

fun ScrollView.setScrollToBottom(isActive: Boolean) {

좋은 의견 감사합니다! 확실히 scrollable이란 표현은 어색하네요..ㅎㅎ 반영하였습니다

다만, 파라미터로 들어온 Boolean 값에 따라 스크롤을 맨 하단으로 강제 이동하는 메서드이므로
파라미터 명은 isScrollable: Boolean 가 더 자연스럽다고 생각해 기존처럼 유지했습니다~!~!

Junyoung-WON · 2024-10-14T07:06:10Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/TextBindingAdapter.kt

+            format(
+                periodFormatString,
+                startAt.year,
+                startAt.monthValue,
+                startAt.dayOfMonth,
+                endAt.year,
+                endAt.monthValue,
+                endAt.dayOfMonth,
+            )


해당 바인딩 어댑터에서 사용하는 format 함수가 okhttp3 라이브러리의 Util.kt 에 존재하는 포맷팅 메서드네요...!
전역에서 편하게 사용하기 위해 만들어진 Util 이지만, okhttp3 와 관련이 없는 BindingAdatpers 에 해당 라이브러리의 import가 존재하는 것이 불필요한 의존성을 만든 것 같아 좋은 구현이라고 생각하지 않습니다.
String의 확장함수로 정의되어있는 String.format() 메서드 등 다른 활용 방안이 많으니, 이로 대체하는 것이 좋아보입니다!

Suggested change

format(

periodFormatString,

startAt.year,

startAt.monthValue,

startAt.dayOfMonth,

endAt.year,

endAt.monthValue,

endAt.dayOfMonth,

)

periodFormatString.format(

startAt.year,

startAt.monthValue,

startAt.dayOfMonth,

endAt.year,

endAt.monthValue,

endAt.dayOfMonth,

)

띠용~ okhttp3의 format import가 숨어있었네요!!
제안해주신대로 String.format()로 변경했습니다 👍

Junyoung-WON · 2024-10-14T07:07:55Z

...ccato_AN/app/src/main/java/com/on/staccato/presentation/bindingadapter/TextBindingAdapter.kt

+            format(
+                periodFormatString,
+                startAt.year,
+                startAt.monthValue,
+                startAt.dayOfMonth,
+                endAt.year,
+                endAt.monthValue,
+                endAt.dayOfMonth,
+            )


해당 코멘트와 같은 의견입니다!

s6m1n added 5 commits October 10, 2024 18:25

refactor: BindingAdapter 파일 분리

232cb85

refactor: ButtonBindingAdapter 리팩토링

9543f70

refactor: ImageBindingAdapter 리팩토링

555ef4b

refactor: TextBindingAdapter 리팩토링

0a1988b

refactor: ExtBindingAdapter 리팩토링

657086d

s6m1n added android We are android>< refactor 리팩토링 (변수 및 메서드 네이밍 변경) labels Oct 10, 2024

s6m1n added this to the sprint-6 milestone Oct 10, 2024

s6m1n requested review from Junyoung-WON and hxeyexn October 10, 2024 13:10

s6m1n self-assigned this Oct 10, 2024

hxeyexn mentioned this pull request Oct 13, 2024

refactor: 도메인명 마이그레이션 #359

Closed

25 tasks

hxeyexn approved these changes Oct 14, 2024

View reviewed changes

s6m1n added 7 commits October 14, 2024 15:29

refactor: 오전, 오후 텍스트 strings.xml로 분리

4261624

refactor: 매직 넘버 상수로 분리

fef0855

style: EtxBindingAdapters -> BindingAdapters.kt

caada85

chore: 불필요한 메서드 제거

97079d3

refactor: 상수화 및 String formatting

623b8c8

fix: 추억 수정 화면 dateRangePicker 중복 버그 해결

88cb3bf

refactor: 추억 기간 선택 시 visibility 바인딩 어댑터 적용

f3ce77f

Junyoung-WON approved these changes Oct 14, 2024

View reviewed changes

refactor: okhttp3 import 제거 및 scrollToBottom rename

cba5569

s6m1n merged commit 3ef1e5d into develop Oct 14, 2024

hxeyexn deleted the feature/#469-binding-adapter-refactor branch October 22, 2024 15:49

Junyoung-WON mentioned this pull request Oct 24, 2024

deploy: v.1.2.0 배포 (잘못) #526

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: BindingAdapter 분리 및 리팩토링 #469 #472

refactor: BindingAdapter 분리 및 리팩토링 #469 #472

s6m1n commented Oct 10, 2024 •

edited

Loading

hxeyexn left a comment

hxeyexn Oct 14, 2024

hxeyexn Oct 14, 2024

s6m1n Oct 14, 2024

hxeyexn Oct 14, 2024

s6m1n Oct 14, 2024 •

edited

Loading

hxeyexn Oct 14, 2024

hxeyexn Oct 14, 2024

s6m1n Oct 14, 2024

hxeyexn Oct 14, 2024

hxeyexn Oct 14, 2024

hxeyexn Oct 14, 2024

hxeyexn Oct 14, 2024

s6m1n Oct 14, 2024

Junyoung-WON left a comment

Junyoung-WON Oct 14, 2024

s6m1n Oct 14, 2024

Junyoung-WON Oct 14, 2024

s6m1n Oct 14, 2024

Junyoung-WON Oct 14, 2024

s6m1n Oct 14, 2024

Junyoung-WON Oct 14, 2024 •

edited

Loading

		val hour = if (setNowDateTime.hour % 12 == 0) 12 else setNowDateTime.hour % 12
		val noonText = if (setNowDateTime.hour < 12) "오전" else "오후"

		@BindingAdapter("scrollableToBottom")
		fun ScrollView.isScrollableToBottom(isScrollable: Boolean) {

refactor: BindingAdapter 분리 및 리팩토링 #469 #472

refactor: BindingAdapter 분리 및 리팩토링 #469 #472

Conversation

s6m1n commented Oct 10, 2024 • edited Loading

⭐️ Issue Number

🚩 Summary

View 별 파일 분리

추억 수정 화면 DatePicker 버그 수정

hxeyexn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s6m1n Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyoung-WON left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Junyoung-WON Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

s6m1n commented Oct 10, 2024 •

edited

Loading

s6m1n Oct 14, 2024 •

edited

Loading

Junyoung-WON Oct 14, 2024 •

edited

Loading