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

친구 검색 화면 구현 #61

Merged
merged 12 commits into from
Dec 18, 2023
Merged

친구 검색 화면 구현 #61

merged 12 commits into from
Dec 18, 2023

Conversation

yonghanJu
Copy link
Contributor

@yonghanJu yonghanJu commented Dec 17, 2023

😎 작업 내용

  • 친구 검색 기능 추가
  • 페이징 제거

🥳 동작 화면

🤯 이슈 번호

🥲 비고

  • 비고 없음

@yonghanJu yonghanJu self-assigned this Dec 17, 2023
@yonghanJu yonghanJu changed the title Compose/community 친구 검색 화면 구현 Dec 17, 2023
@@ -33,8 +34,13 @@ interface FollowService {
@Path("uid") uid: Long,
): Response<List<UserResponse>>

@DELETE(API.UNFOLLOW)
@HTTP(method = "DELETE", path = API.UNFOLLOW, hasBody = true)
Copy link
Member

Choose a reason for hiding this comment

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

오호..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

retorofit GET에서 기본적으로 Body를 못 넣듯, delete 메소드에도 바디를 넣을 수 없었습니다.
따라서 Http 어노테이션으로 바꿔서 진행했습니다!
https://stackoverflow.com/questions/41509195/how-to-send-a-http-delete-with-a-body-in-retrofit

// operator fun invoke(query: String) = otherUserRepository.searchUsers(query).flow
suspend operator fun invoke(query: String): Result<List<UserWithFollowingState>> {
return kotlin.runCatching {
val uid = requireNotNull(accountRepository.uId.first())
Copy link
Member

Choose a reason for hiding this comment

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

저희 uid가 이렇게 사용되는 부분이 반복해서 보이는데 요걸 유즈케이스로 빼서 유즈케이스 주입/사용해도 괜찮을 것 같아요.
그리고 오타가 있네요.
...tory."uId" 부분

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 좋네요

Copy link
Contributor Author

@yonghanJu yonghanJu Dec 18, 2023

Choose a reason for hiding this comment

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

근데 유즈케이스에서 유즈케이스 주입받아서 사용하면 의미 적으로 괜찮을까요??
차라리 Repository 안에 non-null , first 로직을 수행서 uid를 가져오는 함수를 만들어 사용하는건 어떻게 생각하시나요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저희 uid가 이렇게 사용되는 부분이 반복해서 보이는데 요걸 유즈케이스로 빼서 유즈케이스 주입/사용해도 괜찮을 것 같아요. 그리고 오타가 있네요. ...tory."uId" 부분

@soopeach
우선 리뷰된 부분 수정해서 다시 푸시했습니다.
유즈케이스 부분 확인해보시고 말씀하신 부분은 PR로 올려주시면 같이 확인해보고 논의해볼까요

Copy link
Member

Choose a reason for hiding this comment

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

좋습니다..! 요건 나중에 이야기해봐도 좋을 것 같아요.
근데 혹시 유즈 케이스의 의미적 사용이라함은 무엇을 뜻하는 것일까요??
저는 예를 들어, GetUid라는 유즈케이스라면, 이름 그래도 uid를 가져온다는 역할의 유즈케이스이므로 해당 동작을 하는 곳에선 어디에서나 사용해도 무관하다고 생각했어요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋습니다..! 요건 나중에 이야기해봐도 좋을 것 같아요. 근데 혹시 유즈 케이스의 의미적 사용이라함은 무엇을 뜻하는 것일까요?? 저는 예를 들어, GetUid라는 유즈케이스라면, 이름 그래도 uid를 가져온다는 역할의 유즈케이스이므로 해당 동작을 하는 곳에선 어디에서나 사용해도 무관하다고 생각했어요!

저는 UseCase가 View 에서 실행할 수 있는 기능명세의 최소단위라고 생각해서 말씀 드렸습니다!

Comment on lines 142 to 151
// if (lazyPagingItems.loadState.refresh == LoadState.Loading) {
// item {
// Text(
// text = "Waiting for items to load from the backend",
// modifier = Modifier
// .fillMaxWidth()
// .wrapContentWidth(Alignment.CenterHorizontally),
// )
// }
// }
Copy link
Member

Choose a reason for hiding this comment

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

삭제 대신 주석 처리하신 이유가 있으실까요?

Copy link
Contributor Author

@yonghanJu yonghanJu Dec 18, 2023

Choose a reason for hiding this comment

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

서버에서 페이징 API를 제공하지 않아서 기존 코드를 지우고 새롭게 만들었습니다!
이전 코드는 제가 작성한게 아니라 당황하실까봐 남겨뒀는데 확인하셨으니 지워서 다시 푸시하도록 하겠습니다

Copy link
Member

Choose a reason for hiding this comment

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

헉 배려였군요,,, 감사합니다

private val _query = MutableStateFlow("")
Copy link
Member

Choose a reason for hiding this comment

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

저희 String.EMPTY 있지않나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞네요 수정해서 푸시 올리겠습니다

@yonghanJu
Copy link
Contributor Author

확인해보시고 문제 없으면 머지해주세요!

@soopeach soopeach merged commit e372aa5 into compose/develop Dec 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants