-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Feature/#68] university selection api #77
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.
수고하셨습니다
수정 부탁드릴게요
internal abstract class DataSourceModule { | ||
@Binds | ||
@Singleton | ||
abstract fun bindUniversitySelectionDataSource( |
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.
abstract fun bindUniversitySelectionDataSource( | |
abstract fun bindsUniversitySelectionDataSource( |
오타수정해주세요
object ServiceModule { | ||
@Provides | ||
@Singleton | ||
fun provideUniversitySelectionService(@JWT retrofit: Retrofit): UniversitySelectionService = |
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.
여기도 오타 수정해주세요
val dto = UniversitySelectionRequestDto( | ||
universityId = request.universityId, | ||
name = request.name, | ||
longitude = request.longitude, | ||
latitude = request.latitude | ||
) |
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.
이렇게 하지 마시고 mapper사용해주세요
return runCatching { | ||
val response = dataSource.getUniversitySelection() | ||
response.data.universities.map { it.toUniversitySelectionEntity() } | ||
} |
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 University( | ||
@SerialName("id") | ||
val id: Int, | ||
@SerialName("name") | ||
val name: String, | ||
@SerialName("longitude") | ||
val longitude: Double, | ||
@SerialName("latitude") | ||
val latitude: Double | ||
) { | ||
fun toUniversitySelectionEntity() = UniversitySelectionEntity( | ||
id = id, | ||
name = name, | ||
longitude = longitude, | ||
latitude = latitude, | ||
) | ||
} | ||
|
||
@Serializable | ||
data class UniversitySelectionResponseDto( | ||
@SerialName("universities") | ||
val universities: List<University> | ||
) |
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 University( | |
@SerialName("id") | |
val id: Int, | |
@SerialName("name") | |
val name: String, | |
@SerialName("longitude") | |
val longitude: Double, | |
@SerialName("latitude") | |
val latitude: Double | |
) { | |
fun toUniversitySelectionEntity() = UniversitySelectionEntity( | |
id = id, | |
name = name, | |
longitude = longitude, | |
latitude = latitude, | |
) | |
} | |
@Serializable | |
data class UniversitySelectionResponseDto( | |
@SerialName("universities") | |
val universities: List<University> | |
) | |
@Serializable | |
data class UniversitySelectionResponseDto( | |
@SerialName("universities") | |
val universities: List<University> | |
) | |
@Serializable | |
data class University( | |
@SerialName("id") | |
val id: Int, | |
@SerialName("name") | |
val name: String, | |
@SerialName("longitude") | |
val longitude: Double, | |
@SerialName("latitude") | |
val latitude: Double | |
) { | |
fun toUniversitySelectionEntity() = UniversitySelectionEntity( | |
id = id, | |
name = name, | |
longitude = longitude, | |
latitude = latitude, | |
) | |
} |
universities: PersistentList<UniversitySelectionModel>, | ||
selectedUniversity: String?, | ||
onSelectUniversity: (String) -> Unit, | ||
universities: List<UniversitySelectionEntity>, |
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.
List -> PersistantList
isSelected = selectedUniversity == university.name, | ||
onSelectUniversity = onSelectUniversity | ||
isSelected = selectedUniversity == university, | ||
onSelectUniversity = { onSelectUniversity(university) } |
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.
박동민 바보 ㅋㅋ 이거 안대
val universities: List<UniversitySelectionEntity> = emptyList(), |
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.
State에서도 List -> PersistantList 해주세요
).sortedBy { it.name } | ||
_universitySelectionState.value = | ||
_universitySelectionState.value.copy(universities = dummyData) | ||
val result = universitySelectionRepository.getUniversitySelection() |
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.
불필요한 변수 선언하지 말아주세요
val selectedUniversity = _universitySelectionState.value.selectedUniversity | ||
if (selectedUniversity != null) { | ||
val request = UniversitySelectionRequest( | ||
universityId = selectedUniversity.id.toLong(), | ||
name = selectedUniversity.name, | ||
longitude = selectedUniversity.longitude, | ||
latitude = selectedUniversity.latitude | ||
) | ||
viewModelScope.launch { | ||
val result = universitySelectionRepository.postUniversitySelection(request) | ||
result.onSuccess { university -> | ||
val updatedUniversities = | ||
_universitySelectionState.value.universities + university | ||
|
||
_universitySelectionState.value = _universitySelectionState.value.copy( | ||
universities = updatedUniversities | ||
) | ||
}.onFailure { | ||
// 에러 처리 | ||
} | ||
} | ||
} | ||
} |
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.
동멘님이 필요한 부분 다 달아주셔서 달만한 내용이 없네요
저도 리뷰확인하면서 많이 알아갑니다:0
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.
고생하셨습니다~~
역시 채고!!!
⛳️ Work Description
📸 Screenshot
university-selection-apimp4.mp4
📢 To Reviewers