[팀 프로젝트] 첫 코드 리뷰 (2)

이번 글에서는 코드리뷰에서 발견된 문제점과 해결을 위해 코드를
어떻게 수정하였는지에 대한 글을 작성할려고 합니다.

문제점

Model클래스의 생성으로 인한 RecyclerAdatper의 추가적인 구현

abstract class Model(
    open val id : Long,
    open val type : CellType
) {
    companion object {
        val DIFF_CALLBACK = object: DiffUtil.ItemCallback<Model>() {
            override fun areItemsTheSame(oldItem: Model, newItem: Model): Boolean {
                return oldItem.id == newItem.id && oldItem.type == newItem.type
            }

            @SuppressLint("DiffUtilEquals")
            override fun areContentsTheSame(oldItem: Model, newItem: Model): Boolean {
                return oldItem === newItem
            }

        }
    }
}

이후에 만들어질 Model들의 Base가 되는 코드입니다. 코드에서보면 DiffUtil.ItemCallback
유틸리티 클래스를 사용하여 item의 변경된 부분을 찾고 업데이트를 해줍니다. 그렇기에 Difftuil을 사용한다면 사용자에게 변경된 데이터를 바로바로 보여줄 수 있습니다.

팀원이 Base Model 클래스를 이용해 만든 Model 클래스들도 보겠습니다.

abstract class HomeListModel(
    override val id: Long,
    override val type: CellType = CellType.HOME_CELL,
    open val homeListCategory: HomeListCategory
) : Model(id, type) {

    companion object {
        val DIFF_CALLBACK: DiffUtil.ItemCallback<HomeListModel> =
            object : DiffUtil.ItemCallback<HomeListModel>() {
                override fun areItemsTheSame(
                    oldItem: HomeListModel,
                    newItem: HomeListModel
                ): Boolean {
                    return oldItem.id == newItem.id && oldItem.type == newItem.type && oldItem.homeListCategory == newItem.homeListCategory
                }

                @SuppressLint("DiffUtilEquals")
                override fun areContentsTheSame(
                    oldItem: HomeListModel,
                    newItem: HomeListModel
                ): Boolean {
                    return oldItem === newItem
                }
            }
    }
}

 data class TownMarketModel(
    override val id: Long,
    val marketName: String,
    val isMarketOpen: Boolean,
    val locationLatLngEntity: LocationLatLngEntity,
    val marketImageUrl: String,
    val distance: Float, 
    override val type: CellType = CellType.HOME_TOWN_MARKET_CELL
) : Model(id, type)

 data class HomeItemModel(
    override val id: Long,
    val homeListCategory: HomeListCategory,
    val homeListDetailCategory: HomeListDetailCategory,
    val itemImageUrl: String,
    val townMarketModel: TownMarketModel,
    val itemName: String,
    val originalPrice: Int,
    val salePrice: Int,
    val stockQuantity: Int,
    val likeQuantity: Int,
    val reviewQuantity: Int,
    override val type: CellType = CellType.HOME_ITEM_CELL
): Model(id, type) {

팀원은 Model을 상속받은 HomeListModel이라는 클래스를 추가적으로 생성하였고 여기에도 DiffUtil 유틸리티 클래스를 구현했습니다. 추가적인 클래스를 생성한 이유는 화면에서 사용자가 선택을 한 카테고리 별로 item들을 보여주기 위해서 추가적인 클래스를 작성하였다고 하였습니다.

기존의 Model 클래스의 경우 id와 Celltype을 비교하여 데이터를 최신화한다면 팀원이 작성중이였던 화면에서는 각 카테고리에 적합한 데이터의 집어넣기 위해서 Category를 추가적으로 비교가 필요한 상황이였다. 그렇기에 팀원은 HomeListModel이라는 클래스를 새로 생성하여 HomeItemModel이라는 클래스를 만든것입니다. 이 코드만을 본 상황이라면 뭐가 문제지? 라는 생각을 가질 수 있습니다.

HomeListModel안에 DiffUtil이 새로 만들어짐으로써 기존 Model클래스의 DiffUtil 다른 유틸리티클래스가 되어서 별도의 RecyclerAdapter를 만들어줘야한다는 문제점이 발생하였습니다. 문제점을 제시한 조원은 충분히 줄일 수 있는 클래스들 이며 아래의 코드를 제시하였습니다.

문제점을 개선한 코드

abstract class Model(
    open val id : Long,
    open val type : CellType
) {
    open fun isTheSame(item: Model) =
        this.id == item.id && this.type == item.type

    companion object {
        val DIFF_CALLBACK = object: DiffUtil.ItemCallback<Model>() {
            override fun areItemsTheSame(oldItem: Model, newItem: Model): Boolean {
                return oldItem.isTheSame(newItem)
            }

            @SuppressLint("DiffUtilEquals")
            override fun areContentsTheSame(oldItem: Model, newItem: Model): Boolean {
                return oldItem === newItem
            }
        }
    }
}

isTheSame이라는 메소드를 생성하여 Model클래스를 상속받는 다른 Model에서 Override를 하여 필요로하는 데이터의 비교를 할 수 있도록 하였습니다. 또한
BaseModel를 상속받는 것이니 DiffUtil이 같기 때문에 기존의 RecyclerAdapter또한 사이 가능했습니다.=.

data class HomeItemModel(
    override val id: Long,
    override val type: CellType = CellType.HOME_ITEM_CELL
): Model(id, type) {

    /**
     * [Model.isTheSame]을 Override
     */
    override fun isTheSame(item: Model) =
        if (item is HomeItemModel) {
            super.isTheSame(item) && this.homeListCategory == item.homeListCategory
        } else {
            false
        }
}

수정된 Model클래스를 상속받은 HomeItemModel이다. isTHeSame메소드를 Override를 하여 homeListCategory를 비교가 가능하도록 되었습니다.

마치며

누군가는 이렇게 코드를 작성할 수도 있지라는 생각을 하는 반면에 이 코드를 이렇게 변경을 한다면 조금 더 효율적인것 같은데라는 작은 호기심이 프로젝트에 큰 영향을 끼칠 수 있다는 것을 알게되었습니다. 박카스팀에 합류하기 전에 저는 코드의 효율성과 해당 코드 작성의 이유에 대한 생각 보다는 기능의 구현을 우선시에 두고 코드를 작성해 왔습니다. 지금까지 제가 잘못된 방법으로 코딩을 해왔다라는 것을 팀 프로젝트를 함으로써 알게되었으며 클래스를 하나 만들더라도 최적의 코드를 위해 많은 생각과 공부가 필요로 한다는 것을 알게되는 시간이였습니다. 이렇게 첫 코드리뷰를 끝으로 다음 조원의 코드리뷰를 작성하도록 하겠습니다.