2023 05 25 F-Lab 에 안드로이드 멘토님께 저의 Naver API MVVM 패턴을 적용했다고 생각했던 코드릴 리뷰 해주실수 있으신지
궁금해 연락을 드렸다.

결과 : 원래 프로젝트만 코드 리뷰를 해드리는데 한번 볼까요 ? ^^

아 빛 멘토님

정말 감사합니다!!

결과는 멘토님께서 최대한 조심스럽지만 현직자 입장에서 의 코드의 단점을 쏙쏙 빼서 알려주시고 끝나고 사수가 뭐 배웠어 라는 대해

지적사항에 대한 사항을 사수분께 말했더니

2차로 혼났다...

그래도 다시 도전해서 해볼겁니다 !!!

일단 제안 사항

• viewBinding을 쓰셨는데 Data binding 도 써보세요
  필자도 databinding을 알고는 있지만 솔직히 xml에 코드가 들어가는걸 싫어 해서 안쓰고 있었다 허나 databinding을 쓰면 view의 코드가 더욱더 간결해지며 activity 와 fragment은 binding 의 역활만 하기 때문에 더욱더 코드 및 책임성이 수월해 진다 입니다. 그래서 차후 data binding또한 써보도록 하겠습니다.

  지적사항

  • 함수명을 다시 생각해봐요

    fun requestBookApi(id :String , pw : String , query : String) 

  저의 viewModel 의 Naver 책 api 에서 쿼리문을 날리는 viewModel입니다. requestBookApi 라는건 약간 respository 에 있어야 할거 같은 느낌 이네요. 앱의 사용자가 api에 요청을 한다고 생각할까요? 저였으면 searchBook 이라는걸 썼을거 같아요

  • viewModel을 생성할때 어디가 좋을지 생각해봐요

       override fun onCreateView(inflater: LayoutInflater,container: ViewGroup?,savedInstanceState: Bundle?): View? {
        _binding = FragmentSearchBinding.inflate(inflater , container , false )
        initViewModel()
        return binding.root
    }

    /**
     * viewModel init
     */
    private fun initViewModel(){
        val factory = SearchViewModelFactory((requireActivity() as MainActivity).repository)
        _viewModel = ViewModelProvider(this , factory)[SearchViewModel :: class.java]
    }

제가 viewModel을 정의 했던곳은 onCreateView 에서 했지만 안드로이드 생명주기을 잘 보고 어디에 좋을지 다시 생각해봐요

• repostory은 MainActivity에서 저렇게 선언해서 fragment에서 캐스트 해서 쓰는건 안좋은 습관이에요
  왜인지 조사해 오세요
• 여기선 repostoy의 책임이 없어요 그냥 viewModel이 다하고 있네요 다시 짜세요

    fun requestBookApi(id :String , pw : String , query : String) = viewModelScope.launch(Dispatchers.IO) {
        _bookSearchLiveData.postValue(Resource.loading())

        val requestBookApi = repository.requestBookApi(id ,pw , query)
        requestBookApi.enqueue(object: Callback<BookResponse> {
            override fun onResponse(call: Call<BookResponse>, response: Response<BookResponse>) {
                if(response.isSuccessful){ // 200 404 401
                    when(response.code()){
                        Define.SYSTEM_ERROR_NO_API ->{
                            _bookSearchLiveData.postValue(Resource.error(null))
                            return
                        }
                        Define.SYSTEM_ERROR_WRONG_QUERY ->{
                            _bookSearchLiveData.postValue(Resource.fail(Define.MESSAGE_NO_QUERY))
                            return
                        }
                        Define.SYSTEM_ERROR ->{
                            _bookSearchLiveData.postValue(Resource.fail(Define.MESSAGE_SYSTEM_ERROR))
                        }
                        Define.SYSTEM_SUCCESS->{
                            val data = response.body()
                            if(data == null){
                                _bookSearchLiveData.postValue(Resource.fail(Define.MESSAGE_DATA_NULL))
                                return
                            }
                            val result = data.items
                            val saveStatus  = requestSaveSearchList(query)
                            if(!saveStatus){
                                _bookSearchLiveData.postValue(Resource.error(null))
                                return
                            }
                            _bookSearchLiveData.postValue(Resource.success(result))
                        }
                    }
                    return
                }
                _bookSearchLiveData.postValue(Resource.error(null))
            }

            override fun onFailure(call: Call<BookResponse>, t: Throwable) {
                _bookSearchLiveData.postValue(Resource.error(t))
            }
        })
    }
    

전 enque을 viewModel에서 했습니다. 멘토님이 이걸 보시더니 repostory의

class Repository {
    //책 API
    fun requestBookApi(id : String , pw : String , query : String) = RetrofitInterface.requestRetrofit().requestBookApi(id , pw , query )

    //척 검색 기록 api
    fun requestSaveSearchList(data : SearchList) = AppdataBase.getInstance().dataDao().insertSearchList(data)

이 함수는 책임이 없네요. 그리고 이렇게 할거면 차라리 viewModel에서 RetrofitInterface.requestRetrofit().requestBookApi(id , pw , query ) 해도 상관 없잖아요 ...

총 큰건 이것까지다

이제 한번 다시 mvvm 패턴을 다시 파악 해서 짜봐야 겠다