정민욱 assignment3 제출 #57
Conversation
|
|
||
| data class PostWordSetInput(val name: String, val owner: String, val password: String, ) | ||
| data class PutWordInput(val password: String, val word: Word, ) | ||
| data class PasswordInput(val password: String, ) |
There was a problem hiding this comment.
api마다 request body 데이터클래스 이쁘게 만드셨네요👍
data class마다 파일 하나씩 만드셔서 관리하면 더 이쁠 것 같습니다! 파일 하나에 한 줄 밖에 없을 수 있지만, 파일 별로 역할을 나누는 게 더 좋다고 생각해요. RestApi 파일 따로, POST /word_list의 body 파일 따로, POST /word_list의 response 따로 이런 식으로요.
| @POST("/myapp/v1/word_list") | ||
| suspend fun postWordSet( | ||
| @Body wordSet: PostWordSetInput | ||
| ): List<WordSet> |
There was a problem hiding this comment.
response가 List<~~>이라서 data class를 만들 필요가 없을 땐 typealias PostWordSetResponse = List<WordSet>으로 별칭을 정해주시면 예쁩니다!
| private val api: MyRestAPI, | ||
| ): ViewModel() { | ||
|
|
||
| var liveWordSetList: MutableLiveData<List<WordSet>> = MutableLiveData() |
There was a problem hiding this comment.
뷰모델의 LiveData value 변경은 뷰모델 내부에서만 가능해야 하고, 뷰에서는 불가능해야 합니다. 따라서 MutableLiveData는 private하게 만드시고, public한 LiveData를 따로 만드시는 게 좋아요.
private val _liveWordSetList: MutableLiveData<List<WordSet>> = MutableLiveData()
val liveWordSetList: LiveData<List<WordSet>> get() = _liveWordSetList
그리고 변하는 건 LiveData 객체 내부의 value이며 LiveData 객체 자제는 변할 일이 없으므로 var 대신 val을 쓰시는 게 좋습니다.
| private var password: String = "" | ||
|
|
||
| fun getWordSetListFromServer() { | ||
| viewModelScope.launch(Dispatchers.IO) { |
There was a problem hiding this comment.
viewModel의 메서드까지 suspend fun으로 하시고, 뷰에서 lifecycleScope.launch 등으로 실행하시는 게 좋다고 생각합니다.
사실 이렇게 일반 함수와 viewModelScope.launch를 이용하셔도 동작에 큰 차이는 없지만, api 호출이 성공적일 때만 다이얼로그를 닫는다든지, 에러가 발생하면 view에서 바로 Toast를 띄운다든지 등의 동작을 더 편하게 할 수 있습니다.
|
|
||
| fun postWordSetToServer(input: PostWordSetInput) { | ||
| viewModelScope.launch(Dispatchers.IO) { | ||
| if (input.name == "" || input.owner == "" || input.password == "") { |
There was a problem hiding this comment.
[nit] String의 메서드인 isNotEmpty()나 isEmpty()를 쓰시면 가독성을 높일 수 있습니다!
|
|
||
| binding.backButton.setOnClickListener { | ||
| val intent = Intent(this@DetailActivity, MainActivity::class.java) | ||
| startActivity(intent) |
There was a problem hiding this comment.
뒤로가기를 눌렀을 때 새로운 MainActivity를 만들기보다는 현재 액티비티를 종료해주세요! 액티비티를 시작하는 건 상당히 큰 작업이며, 여러 액티비티가 계속 쌓이며 리소스를 낭비하게 됩니다. 또한 DetailActivity를 여러번 시작한 후 MainActivity에서 시스템 버튼으로 뒤로가기하면 바탕화면으로 나가지지 않고 이전 액티비티가 보이게 됩니다.
No description provided.