[Refactor/#427] BrowseWorkbooksUseCase 리펙토링 #428

belljun3395 · 2024-09-28T12:19:19Z

🎫 연관 이슈

resolved: #427

💁‍♂️ PR 내용

BrowseWorkbooksUseCase 리펙토링

🙏 작업

WorkbookOrderDelegator 구현체 이동 및 시그니처 변경
WorkBooks, OrderedWorkBooks 모델 생성

🙈 PR 참고 사항

📸 스크린샷

🤖 테스트 체크리스트

체크 미완료
체크 완료

belljun3395 · 2024-09-28T12:20:20Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/WorkBooks.kt

+open class WorkBooks(
+    private val workbooks: List<WorkBook>,
+) {


WorkBook 리스트로 생성하는 WorkBooks 모델을 추가하였습니다.
해당 모델로는 WorkBook 리스트를 조회할 조회할 수 없도록 private val로 선언하였습니다.

belljun3395 · 2024-09-28T12:21:48Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/WorkBooks.kt

+    fun order(delegator: WorkbookOrderDelegator): OrderedWorkBooks {
+        return delegator.order(workbooks)
+    }


정렬시에는 Workbooks가 가지고 있는 Workbook 리스트와 비교할 데이터가 필요할 수 있다고 생각하였습니다.
그래서 WorkbookOrderDelegator를 생성할 때 비교할 데이터와 함께 생성하고 order 메서드에는 Workbooks가 가지고 있는 Workbook 리스트 넘겨 정렬된 워크북(OrderedWorkbooks)을 반환받을 수 있도록 하였습니다.

이해몬함..

아 저도 코멘트 달면서 생각하는 것을 제대로 전달 할 수 있을까 생각했던 것 같아요
남겨주신 코멘트 반영해서 추가 수정한 이후에 별도의 글 + 그림으로 쭉 설명하는 것을 추가할께요!

belljun3395 · 2024-09-28T12:22:22Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/order/WorkbookOrderDelegator.kt

+     * @param workbooks 정렬할 워크북 목록
+     * @return 정렬된 워크북 목록
+     */
+    fun order(workbooks: List<WorkBook>): OrderedWorkBooks


WorkbookDelegator의 order 메서드에선 비교 원본을 파라미터로 받고 정렬후 OrderedWorkBooks를 반환합니다.

belljun3395 · 2024-09-28T12:23:11Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/OrderedWorkBooks.kt

+class OrderedWorkBooks(
+    val workbooks: List<WorkBook>,
+) : WorkBooks(workbooks)


정렬이 완료된 Workbook 리스트가 있는 OrderedWorkBooks은 workbooks를 노출합니다.
WorkBooks는 workbooks를 노출하지 않는다.

이미 정렬된 상태라는걸 의미할 수 있게 workbooks 변수명 수정하는것도 좋을거 같아요

OrderedWorkBooks가 WorkBooks를 상속하는데, OrderedWorkBooks는 이미 정렬이 되어 있는애들이기 때문에, 얘에 대해 order 메소드를 실행하면 어떻게 되나요?

음.. 구현 당시에는 OrderedWorkBooks도 다시 정렬해도 괜찮을 것이라 생각했는데 조금 더 분리하는 방법을 고민해 볼께요!

OrderedWorkBooks가 WorkBooks를 상속하는데, OrderedWorkBooks는 이미 정렬이 되어 있는애들이기 때문에, 얘에 대해 order 메소드를 실행하면 어떻게 되나요?

이번 구현에 정렬되지 않은 상태의 클래스만 정렬을 수행할 수 있도록 수정하였습니다.

belljun3395 · 2024-09-28T12:24:10Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-                subscriptionCount = record.subscriptionCount
-            )
-        }
+        val workbooks = toWorkbooks(workbookRecords, writerRecords)


값들을 단순 조합해서 새로운 객체(Workbook, Workbooks)를 만들기에 toWorkbooks라 네이밍하였습니다.

해당 변환 과정을 UC에서 하는게 아니라 WorkBooks 모델 자체에서 제공해주는건 어떤가요

컨버터를 만들어서 할 수 있을 것 같은데
저희가 부분적으로 모델을 사용하고 있어서 우선 메서드를 선택했어요
추후 모델이 조금 더 많아지면 도입하는게 어떤가요?

belljun3395 · 2024-09-28T12:24:58Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-        }
-
-        val orderedWorkbooks = when (orderStrategy) {
+        val orderStrategy = getOrderStrategy(useCaseIn)


useCaseIn의 값을 가지고 단순 판단만 들어가 getOrderStrategy라 네이밍 하였습니다.

belljun3395 · 2024-09-28T12:25:55Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-        }.let { delegator ->
-            workbookOrderDelegatorExecutor.execute(delegator)
+            else -> genBasicWorkbookOrderDelegator()
        }

-        orderedWorkbooks.map { workBook ->
+        val orderedWorkbooks = workbooks.order(orderDelegator)


이전과 다르게 workbooks 객체에 workbookOrderDelegator를 넘기고
workbooks 객체가 order을 진행한 이후 orderedworkbooks 객체를 반환받습니다.

넵 모델 자체적으로 진행하는게 좋아보이네요

hun-ca

수고많으십니다

hun-ca · 2024-09-29T08:36:24Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

+    private fun toWorkbooks(
+        workbookRecords: List<SelectWorkBookRecordWithSubscriptionCount>,
+        writerRecords: Map<Long, List<WriterMappedWorkbookOutDto>>,


SelectWorkBookRecordWithSubscriptionCount -> 디비에서 조회된 결과를 record로 끝나기로 약속되어 있는데 네이밍이 약간 어색해보임.

writerRecords -> 실제 받는 타입이 레코드가 아닌데 변수명은 왜 record인지?

요 부분은 인텔리 자동완성에 너무 의지한 것 같네요..!
적절한 네이밍은 고민해볼께요!

디비에서 조회된 결과를 record로 끝나기로 약속되어 있는데 네이밍이 약간 어색해보임.

다시보니 파라미터로 받는 둘다 디비에서 조회된 결과를 받는 것이라 record로 해도 괜찮을 것 같은데
혹시 다른 적절한 파라미터 네이밍 있을까요?

hun-ca · 2024-09-29T08:38:22Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-                subscriptionCount = record.subscriptionCount
-            )
-        }
+        val workbooks = toWorkbooks(workbookRecords, writerRecords)


해당 변환 과정을 UC에서 하는게 아니라 WorkBooks 모델 자체에서 제공해주는건 어떤가요

hun-ca · 2024-09-29T08:41:18Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-        }.let { delegator ->
-            workbookOrderDelegatorExecutor.execute(delegator)
+            else -> genBasicWorkbookOrderDelegator()
        }

-        orderedWorkbooks.map { workBook ->
+        val orderedWorkbooks = workbooks.order(orderDelegator)


넵 모델 자체적으로 진행하는게 좋아보이네요

hun-ca · 2024-09-29T08:43:11Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/OrderedWorkBooks.kt

+class OrderedWorkBooks(
+    val workbooks: List<WorkBook>,
+) : WorkBooks(workbooks)


이미 정렬된 상태라는걸 의미할 수 있게 workbooks 변수명 수정하는것도 좋을거 같아요

hun-ca · 2024-09-29T08:55:49Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/WorkBooks.kt

+    fun order(delegator: WorkbookOrderDelegator): OrderedWorkBooks {
+        return delegator.order(workbooks)
+    }


이해몬함..

hun-ca · 2024-09-29T08:59:13Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/OrderedWorkBooks.kt

+class OrderedWorkBooks(
+    val workbooks: List<WorkBook>,
+) : WorkBooks(workbooks)


OrderedWorkBooks가 WorkBooks를 상속하는데, OrderedWorkBooks는 이미 정렬이 되어 있는애들이기 때문에, 얘에 대해 order 메소드를 실행하면 어떻게 되나요?

belljun3395

리뷰 감사합니다.
추가 리펙한 한 이후에 다시 리뷰 요청해볼께요!

belljun3395 · 2024-09-29T09:32:59Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-                subscriptionCount = record.subscriptionCount
-            )
-        }
+        val workbooks = toWorkbooks(workbookRecords, writerRecords)


컨버터를 만들어서 할 수 있을 것 같은데
저희가 부분적으로 모델을 사용하고 있어서 우선 메서드를 선택했어요
추후 모델이 조금 더 많아지면 도입하는게 어떤가요?

belljun3395 · 2024-09-29T09:33:58Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

+    private fun toWorkbooks(
+        workbookRecords: List<SelectWorkBookRecordWithSubscriptionCount>,
+        writerRecords: Map<Long, List<WriterMappedWorkbookOutDto>>,


요 부분은 인텔리 자동완성에 너무 의지한 것 같네요..!
적절한 네이밍은 고민해볼께요!

belljun3395 · 2024-09-29T09:34:55Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/OrderedWorkBooks.kt

+class OrderedWorkBooks(
+    val workbooks: List<WorkBook>,
+) : WorkBooks(workbooks)


음.. 구현 당시에는 OrderedWorkBooks도 다시 정렬해도 괜찮을 것이라 생각했는데 조금 더 분리하는 방법을 고민해 볼께요!

belljun3395 · 2024-09-29T09:36:20Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/model/WorkBooks.kt

+    fun order(delegator: WorkbookOrderDelegator): OrderedWorkBooks {
+        return delegator.order(workbooks)
+    }


아 저도 코멘트 달면서 생각하는 것을 제대로 전달 할 수 있을까 생각했던 것 같아요
남겨주신 코멘트 반영해서 추가 수정한 이후에 별도의 글 + 그림으로 쭉 설명하는 것을 추가할께요!

belljun3395 · 2024-09-30T02:49:11Z

수정 구현한 클래스 다이어그램

belljun3395 · 2024-09-30T02:50:35Z

기존 구현의 흐름

수정 구현의 흐름

belljun3395 · 2024-09-30T02:51:47Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-            useCaseIn.viewCategory == ViewCategory.MAIN_CARD && useCaseIn.memberId == null -> WorkBookOrderStrategy.MAIN_VIEW_UNAUTH
-            else -> WorkBookOrderStrategy.BASIC
-        }
+        val workbooks = toWorkbooks(workbookRecords, writerRecords)


우선 workbooks 데이터로 만듭니다.

belljun3395 · 2024-09-30T02:52:25Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-                    )
-                }.let { subscribedWorkbooks ->
-                    AuthMainViewWorkbookOrderDelegator(workbookDetails, subscribedWorkbooks)
+        val orderedWorkbook = OrderTargetWorkBooks(workbooks).let { target ->


order 모델에서 사용할 OrderTargetWorkBooks로 변환합니다.

belljun3395 · 2024-09-30T02:53:03Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

+            orderDelegator
+                ?.let { delegator ->
+                    UnOrderedWorkBooks(
+                        target,
+                        delegator
+                    ).order().orderedWorkbooks
                }


orderDelegator가 있으면 변환할 필요가 있다고 판단하고 UnOrderedWorkBooks를 만든 이후 정렬합니다.

belljun3395 · 2024-09-30T02:55:14Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

+            orderDelegator
+                ?.let { delegator ->
+                    UnOrderedWorkBooks(
+                        target,
+                        delegator
+                    ).order()
                }


orderDelegator 가 존재한다면 정렬 되어있지 않다고 판단하고 UnOrderedWorkBooks를 생성한 후 정렬하여 OrderedWorkBooks를 반환합니다.

belljun3395 · 2024-09-30T02:55:46Z

api/src/main/kotlin/com/few/api/domain/workbook/usecase/BrowseWorkbooksUseCase.kt

-        }.let { delegator ->
-            workbookOrderDelegatorExecutor.execute(delegator)
-        }
+                ?: run { OrderedWorkBooks(target) }


orderDelegator 가 존재하지 않는다면 정렬된 상태라 판단하고 바로 OrderedWorkBooks를 반환합니다.

belljun3395 added 6 commits September 28, 2024 21:14

feat: Workbooks 모델 생성

052072f

refactor: 기존 WorkbookOrderDelegator 이동 및 order 메서드 시그니쳐 변경

fe48a91

refactor: WorkbookOrderDelegator 구현체 이동 및 변경 반영

838f7f2

feat: OrderedWorkBooks 모델 생성

92e0f68

refactor: BrowseWorkbooksUseCase에 변경 반영

6f79d97

refactor: 변경 사항 테스트에 반영

ee3acee

belljun3395 requested a review from hun-ca as a code owner September 28, 2024 12:19

github-actions bot added the refactor 기존 기능에 대해 개선할 때 사용됩니다. label Sep 28, 2024

github-actions bot assigned belljun3395 Sep 28, 2024

belljun3395 commented Sep 28, 2024

View reviewed changes

hun-ca approved these changes Sep 29, 2024

View reviewed changes

belljun3395 commented Sep 29, 2024

View reviewed changes

belljun3395 added 5 commits September 30, 2024 11:45

refactor: WorkBook 클래스 범용 데이터 클래스로 변경

55a1c04

feat: order 모델에서 사용할 OrderTargetWorkBooks 구현

11a473d

feat: order 모델 부모 객체 생성

a43310f

refactor: order 모델 변경 사항 반영

3d67632

refactor: 변경 사항 반영

433c3ba

refactor: orderedWorkbooks 공통으로 처리할 수 있도록 수정

60f3388

belljun3395 commented Sep 30, 2024

View reviewed changes

fix: 변경 사항 테스트에 적용

d24e5d3

belljun3395 merged commit 605cb07 into dev Oct 10, 2024
8 checks passed

[Refactor/#427] BrowseWorkbooksUseCase 리펙토링 #428

[Refactor/#427] BrowseWorkbooksUseCase 리펙토링 #428

Conversation

belljun3395 commented Sep 28, 2024

🎫 연관 이슈

💁‍♂️ PR 내용

🙏 작업

🙈 PR 참고 사항

📸 스크린샷

🤖 테스트 체크리스트

Choose a reason for hiding this comment

belljun3395 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

belljun3395 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

belljun3395 Sep 28, 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

belljun3395 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

belljun3395 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

belljun3395 Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hun-ca 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

belljun3395 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

belljun3395 commented Sep 30, 2024

수정 구현한 클래스 다이어그램

belljun3395 commented Sep 30, 2024

기존 구현의 흐름

수정 구현의 흐름

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

belljun3395 Sep 28, 2024 •

edited

Loading

belljun3395 Sep 28, 2024 •

edited

Loading

belljun3395 Sep 28, 2024 •

edited

Loading

belljun3395 Sep 28, 2024 •

edited

Loading

belljun3395 Sep 28, 2024 •

edited

Loading

belljun3395 Sep 28, 2024 •

edited

Loading