Skip to content
This repository has been archived by the owner on May 19, 2024. It is now read-only.

[WEAV-329] 이전 채팅메시지 조회 API 구현 #239

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Conversation

waterfogSW
Copy link
Member

@waterfogSW waterfogSW added the feat New feature or request label Apr 13, 2024
@waterfogSW waterfogSW self-assigned this Apr 13, 2024

fun getScrollList(query: ScrollListQuery): ScrollListResult

data class ScrollListQuery(
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 이거 상속 말고 그냥 제너릭만 써서 하면 좀 그럴까요?
딱히 여기서 문제는 아님

Copy link
Member Author

Choose a reason for hiding this comment

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

상속말고 제너릭만 사용하면 어떤 방식으로 코드가 작성되나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 느낌으로 제너릭만 설정해주면 해당 값만 잘 맞춰서 넣어줄 수 있도록?

Suggested change
data class ScrollListQuery(
interface GetChatMessage {
fun getScrollList(query: ScrollRequest<UUID>): ScrollResponse<ChatMessage, UUID?>
}

Copy link
Member Author

Choose a reason for hiding this comment

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

음 그렇게 하면 roomId같은 속성은 추가로 파라미터 더 만들어줘야하는데 그게 더 낫다고 생각하시나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

잘 이해를 못 했어요! UUID가 chatRoomId 아니에요? 어차피 들어오는 cursor 역할 하는 친구의 타입만 바뀌는 거 같아서요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 제안하고 싶은 건 지금은 거의 사실 Scroll관련 요청에서 next조건이 되는 타입 필드만 정의해서 쓰는 정도라 굳이 상속 안하고 제너릭으로 하고 클래스에 잘 미리 정의해서 재활용하는게 낫지 않나에요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

밑에 ScrollListResult도 그냥 ScrollResponse<CHatMessage, UUID?> 를 그대로 써도 될 것 같아서요!

Copy link
Member Author

Choose a reason for hiding this comment

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

어 근데 다른 ScrollResponse, ScrollRequest 활용케이스들은 모두 상속해서 정의하고 있는데, 이부분에 대해서만 예외적으로 적용하는게 일관성이 좀 깨질것같은데, 어떻게 생각하시나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

네네 맞아요 그래서 괜찮으면 한번에 리팩토링 할까 생각했어요!

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 요거는 나중에 조금더 논의하면 좋을거같아요

Copy link

@waterfogSW waterfogSW merged commit cb2ce03 into main Apr 14, 2024
3 checks passed
@waterfogSW waterfogSW deleted the feat/WEAV-329 branch April 14, 2024 07:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants