-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FEAT/#117] 장소 검색 API 세팅 #121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
컨벤션과 다른 부분이 있어 이 부분만 반영 해주세요~
@Serializable | ||
data class BaseResponse<T>( | ||
@SerialName("success") | ||
val success: Boolean, | ||
@SerialName("error") | ||
val error: ErrorResponse? = null, | ||
@SerialName("data") | ||
val data: T? = null | ||
) | ||
|
||
@Serializable | ||
data class ErrorResponse( | ||
@SerialName("message") | ||
val message: String | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4) 추후 세홍이형 PR 머지하게되면 충돌 날 것 같아요!
꼭 해결하고 머지부탁드려요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네ㅎ 컨플릭 해결하겠습니다!
fun PlaceDto.toPlaceEntity(): PlaceEntity = | ||
PlaceEntity( | ||
placeName = placeName, | ||
placeAddress = placeAddress, | ||
placeRoadAddress = placeRoadAddress, | ||
latitude = latitude, | ||
longitude = longitude | ||
) | ||
|
||
fun PlaceEntity.toPlace(): Place = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p2) 함수명이 저희 컨벤션과 다른 것 같습니다! 수정 해주세요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코딩 컨벤션과 맞게 수정했습니다~
override suspend fun searchPlace(query: String, display: Int): Result<List<Place>> = runCatching { | ||
val response = placeDataSource.searchPlace(query, display) | ||
if (!response.success || response.data == null) { | ||
throw IllegalStateException(response.error?.message ?: "에러 발생") | ||
} | ||
response.data.placeList.map { placeDto -> | ||
placeDto.toPlaceEntity().toPlace() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p1) 저희 컨벤션 맞게 runCatching 내부에서 !!
를 사용하는 방법으로 부탁드려요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override suspend fun searchPlace(query: String, display: Int): Result<List<Place>> = runCatching {
placeDataSource.getPlaces(query, display).data!!.placeList
.map { it.toDomain().toPresentation() }
}
로 수정했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
캬~ 수고하셨습니다😊😊
import com.spoony.spoony.domain.entity.PlaceEntity | ||
import com.spoony.spoony.presentation.register.model.Place | ||
|
||
fun PlaceDto.toPlaceEntity(): PlaceEntity = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 저희 컨벤션에 맞게 함수명 고쳐주세요~~!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니당~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 컨벤션에 맞게 네이밍 수정해봅시다~~ 노션 읽어라~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
히히 수정했습니다요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잡았다!!ㅋㅋㅋㅋ
import com.spoony.spoony.presentation.register.model.Category | ||
import com.spoony.spoony.presentation.register.model.Place |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 어라라????? data레이어에서 presentation 레이어의 모델을 사용하네요?? 수정해주세요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
싹 수정했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~🚀
…arch-api # Conflicts: # app/src/main/java/com/spoony/spoony/data/di/DataSourceModule.kt # app/src/main/java/com/spoony/spoony/data/di/ServiceModule.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최고~~ 👍
Category( | ||
categoryId = entity.categoryId, | ||
categoryName = entity.categoryName, | ||
iconUrlSelected = entity.iconUrl, | ||
iconUrlNotSelected = entity.unSelectedIconUrl ?: entity.iconUrl | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4) Category를 선언한 곳에서 mapper를 만들어 주는 방법도 있답니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어푸푸!!🚀🚀 그래도 코리 확인 후에 머지 해주세요!!
Place( | ||
placeName = entity.placeName, | ||
placeAddress = entity.placeAddress, | ||
placeRoadAddress = entity.placeRoadAddress, | ||
latitude = entity.latitude, | ||
longitude = entity.longitude | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: mapper 함수를 만들어주는건 어떤가요??
searchResults = places.toImmutableList(), | ||
isSearching = false | ||
.onSuccess { placeEntities -> | ||
val places = placeEntities.map { entity -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: value로 한번 저장해주는 이유가 있으신가요?? 가능하면 객체로 저장하지 말고 바로 state에 넣어주면 좋을 것 같아요!
Related issue 🛠
Work Description ✏️
To Reviewers 📢
Entity 만들어서 사용하시면 됩니다.