refactor : EventDetail 코드 리팩토링 #282

chws0508 · 2023-08-10T08:46:01Z

#️⃣연관된 이슈

#277

📝작업 내용

EventDetail 부분 포맷팅 맞췄어요

스크린샷 (선택)

예상 소요 시간 및 실제 소요 시간

1시간/1시간

💬리뷰 요구사항(선택)

EventDetail 부분만 봐주셨으면 좋겠어요 ㅎㅎ

ki960213 · 2023-08-10T08:47:16Z

android/2023-emmsale/app/src/main/java/com/emmsale/data/eventdetail/EventDetail.kt

-    val startDate: String,
-    val endDate: String,
+    val startDate: LocalDateTime,
+    val endDate: LocalDateTime,


데이터 모델에서 날짜 표현을 위해 String에서 LocalDateTime으로 타입을 바꾼 것 좋네요 👍

ki960213 · 2023-08-10T08:48:23Z

android/2023-emmsale/app/src/main/java/com/emmsale/data/eventdetail/dto/EventDetailApiModel.kt

@@ -3,6 +3,8 @@ package com.emmsale.data.eventdetail.dto
 import com.emmsale.data.eventdetail.EventDetail
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
+import java.time.LocalDateTime
+import java.time.format.DateTimeFormatter

 @Serializable
 data class EventDetailApiModel(


ApiModel을 다른 레이어의 모델로 변경하는 작업은 Mapper에서 하기로 컨벤션을 결정했던 것 같습니다! 😄

아 맞네요 ㅎㅎ깜빡했어요 감사요

ki960213 · 2023-08-10T09:06:28Z

...023-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailActivity.kt

@@ -71,7 +72,7 @@ class EventDetailActivity : AppCompatActivity() {
        text = tag
    }

-    private fun setBackPress() {
+    private fun onBackPressButtonClick() {


위로 가기 버튼을 클릭했을 때 실행되어야 할 작업 모음을 정의하는 함수의 이름이라면 onBackPressButtonClick이라는 이름이 괜찮다고 생각합니다! 하지만 위 함수는 위로 가기 버튼이 클릭 이벤트가 발생했을 때 처리하는 일을 하는 OnClickListener를 변경하므로 initBackPressOnClickListener 이렇게 이름을 짓는 것이 더 정확하다고 생각합니다:)
저라면 initUpButton 혹은 setupUpButton 이렇게 지을 것 같습니다 :)

저는 특정 View에 ClickListener를 설정할 때 initXxxClickListener() 라는 메서드명을 자주 사용합니다.

인정이요 반영할게요

…ature/#277-스캇_코드_리팩토링

tmdgh1592 · 2023-08-10T13:16:35Z

android/2023-emmsale/app/src/main/java/com/emmsale/data/eventdetail/EventDetail.kt

-    val startDate: String,
-    val endDate: String,
+    val startDate: LocalDateTime,
+    val endDate: LocalDateTime,
    val informationUrl: String,
    val tags: List<String>,
    val imageUrl: String?,


imageUrl 대신에 posterImageUrl은 어떤가요?
imageUrl이라고 하면 어떤 이미지의 Url인지 파악하기가 어려울 수 있을 것 같아요!

tmdgh1592 · 2023-08-10T13:27:46Z

...023-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailActivity.kt

+        binding.lifecycleOwner = this
+        binding.vm = viewModel


이 코드는 setContentView 이후에 호출해주는 것도 좋을 것 같아요.
viewModel이 초기화되면서 실행되는 로직이 있다면 버그가 발생할 수 있습니다.

확인이요

tmdgh1592 · 2023-08-10T13:29:54Z

...023-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailActivity.kt

+            if (eventDetailUiState.isError) {
+                showToast(getString(R.string.eventdetail_fetch_eventdetail_error_message))
+            } else {
+                addTag(eventDetailUiState.tags)


어떤 태그를 추가하는 메서드인지 구체적으로 명명해주시면 좋을 것 같아요!

확인이요

tmdgh1592 · 2023-08-10T13:41:18Z

...023-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailActivity.kt

@@ -71,7 +72,7 @@ class EventDetailActivity : AppCompatActivity() {
        text = tag
    }

-    private fun setBackPress() {
+    private fun onBackPressButtonClick() {


저는 특정 View에 ClickListener를 설정할 때 initXxxClickListener() 라는 메서드명을 자주 사용합니다.

tmdgh1592 · 2023-08-10T13:42:35Z

...23-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailViewModel.kt

-    private val _eventDetail: MutableLiveData<EventDetailUiState> =
-        MutableLiveData<EventDetailUiState>()
-    val eventDetail: LiveData<EventDetailUiState>
+    private val _eventDetail: NotNullMutableLiveData<EventDetailUiState> =


_eventDetail의 타입을 명시해주지 않으면 한 줄에 깔끔하게 작성할 수 있어요!

저는 무조건 타입을 명시해야 한다고 생각합니다!

어떤 타입인지 명확하게 적는 것이 실수를 방지한다고 생각해요

tmdgh1592 · 2023-08-10T13:43:07Z

...23-emmsale/app/src/main/java/com/emmsale/presentation/ui/eventdetail/EventDetailViewModel.kt

+    val eventDetail: NotNullLiveData<EventDetailUiState>
        get() = _eventDetail


다음과 같이 작성해보는 것은 어떠신가요?
val eventDetail: NotNullLiveData<EventDetailUiState>= _eventDetail

tmdgh1592 · 2023-08-10T14:00:09Z

android/2023-emmsale/app/src/main/res/values/strings.xml


    <string name="conference_default_people_count">0</string>
+    <string name="eventdetail_fetch_eventdetail_error_message">이벤트를 불러올 수 없어요</string>
+    <string name="eventdetail_tab_infromation">상세 정보</string>
+    <string name="eventdetail_tab_comment">댓글</string>
+    <string name="eventdetail_tab_recruitment">같이가요</string>


conference_default_people_count과 한 칸 띄어주세요~

- 함수명 변경 - strings.xml 포맷 변경 - initFragmentStateAdapter 리스트 생성하여 attach 하도록 변경 - imageUrl 을 postImageUrl 로 좀더 명확하게 변경

tmdgh1592

리뷰 반영 고생하셨습니다!
approve 하겠습니다 : )

ki960213

리뷰를 잘 반영해주셨네요 👍
더 이상 리뷰할 게 없는 것 같습니다!

refactor : EventDetail 코드 리팩토링

35a1998

chws0508 added the Android 안드로이드 관련 이슈 label Aug 10, 2023

chws0508 added this to the 4차 스프린트 1주차 milestone Aug 10, 2023

chws0508 self-assigned this Aug 10, 2023

chws0508 requested review from ki960213 and tmdgh1592 and removed request for ki960213 August 10, 2023 08:46

chws0508 added 2 commits August 10, 2023 17:48

refactor : EventDetail 코드 리팩토링

194191d

refactor : EventDetail 코드 리팩토링

b4c4b40

ki960213 reviewed Aug 10, 2023

View reviewed changes

chws0508 added 5 commits August 10, 2023 22:48

refactor : EventDetail Mapper 분리

7bf4d1c

refactor : 버튼 클릭 함수명 변경

bbd4406

refactor : viewModel 생성시에 한번만 fetch 하도록 수정

811782f

Merge remote-tracking branch 'origin/Feature/#277-스캇_코드_리팩토링' into Fe…

0e92d64

…ature/#277-스캇_코드_리팩토링

refactor : mapper 분리에 대한 RepositoryImpl 변경사항 적용

111d9dc

tmdgh1592 requested changes Aug 10, 2023

View reviewed changes

chws0508 added 2 commits August 11, 2023 10:18

refactor : init 함수와 프로퍼티 순서 바꿈

9a1b092

refactor : 부나 피드백 반영

5087a0a

- 함수명 변경 - strings.xml 포맷 변경 - initFragmentStateAdapter 리스트 생성하여 attach 하도록 변경 - imageUrl 을 postImageUrl 로 좀더 명확하게 변경

tmdgh1592 approved these changes Aug 11, 2023

View reviewed changes

ki960213 approved these changes Aug 11, 2023

View reviewed changes

refactor : EventDetailApiModel 속성 변경

6fbb8b1

chws0508 merged commit c53f59e into android-main Aug 11, 2023

chws0508 deleted the Feature/#277-스캇_코드_리팩토링 branch August 11, 2023 07:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor : EventDetail 코드 리팩토링 #282

refactor : EventDetail 코드 리팩토링 #282

chws0508 commented Aug 10, 2023

ki960213 Aug 10, 2023

chws0508 Aug 10, 2023

ki960213 Aug 10, 2023

chws0508 Aug 10, 2023

ki960213 Aug 10, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 10, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 11, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 11, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 11, 2023

tmdgh1592 Aug 10, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 11, 2023

chws0508 Aug 11, 2023

tmdgh1592 Aug 10, 2023

chws0508 Aug 11, 2023

tmdgh1592 Aug 10, 2023

tmdgh1592 left a comment

ki960213 left a comment

		val eventDetail: NotNullLiveData<EventDetailUiState>
		get() = _eventDetail

refactor : EventDetail 코드 리팩토링 #282

refactor : EventDetail 코드 리팩토링 #282

Conversation

chws0508 commented Aug 10, 2023

#️⃣연관된 이슈

📝작업 내용

스크린샷 (선택)

예상 소요 시간 및 실제 소요 시간

💬리뷰 요구사항(선택)

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmdgh1592 left a comment

Choose a reason for hiding this comment

ki960213 left a comment

Choose a reason for hiding this comment