어떻게 함수를 써야 올바른 or 깔끔한 코드일까요?

Question

코드를 좀 더 간결하게 하고싶어 apply, run , with 등과같은 함수를 사용하려는데

셋다 정상동작하니 어떠한것이 올바른지 잘모르겠습니다.

어떤게 옳은가요?

오리지널 기존 코드

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {
    holder.title.text = items[position].title + " $position"

    holder.nestedRV.apply {
        layoutManager = GridLayoutManager(
            holder.nestedRV.context,
            5
        )
        adapter = ChildAdapter(items[position].child)
        setRecycledViewPool(viewPool)
    }

    holder.nestedRV.visibility = if(items[position].isExpanded) View.VISIBLE else View.GONE
    holder.borderline.visibility = if(items[position].isExpanded) View.VISIBLE else View.GONE
    holder.expanded.setOnClickListener {
        items[position].isExpanded = !items[position].isExpanded
        notifyDataSetChanged() // 여기에 사용하는 것이 맞나?
    }
}

 

apply 사용해서 정리했을때.

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {
    holder.title.text = items[position].title + " $position"

//        holder.nestedRV.apply {
//            layoutManager = GridLayoutManager(
//                holder.nestedRV.context,
//                5
//            )
//            adapter = ChildAdapter(items[position].child)
//            setRecycledViewPool(viewPool)
//        }

    holder.apply {
        nestedRV.apply {
            layoutManager = GridLayoutManager(
                holder.nestedRV.context,
                5
            )
            adapter = ChildAdapter(items[position].child)
            setRecycledViewPool(viewPool)
        }
        
        nestedRV.visibility = if(items[position].isExpanded) View.VISIBLE else View.GONE
        borderline.visibility = if(items[position].isExpanded) View.VISIBLE else View.GONE

        items[position].apply {
            nestedRV.visibility = if(isExpanded) View.VISIBLE else View.GONE
            borderline.visibility = if(isExpanded) View.VISIBLE else View.GONE
            expanded.setOnClickListener {
                isExpanded = !isExpanded
                notifyDataSetChanged()
            }
        }
    }
}

 

run 사용1

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {
    holder.title.text = items[position].title + " $position"

//    holder.nestedRV.run {
//        layoutManager = GridLayoutManager(
//            holder.nestedRV.context,
//            5
//        )
//        adapter = ChildAdapter(items[position].child)
//        setRecycledViewPool(viewPool)
//    }


    holder.run {
        items[position].run {
            nestedRV.layoutManager = GridLayoutManager(
                holder.nestedRV.context,
                5
            )
            nestedRV.adapter = ChildAdapter(items[position].child)
            nestedRV.setRecycledViewPool(viewPool)
            nestedRV.visibility = if(isExpanded) View.VISIBLE else View.GONE
            borderline.visibility = if(isExpanded) View.VISIBLE else View.GONE
            expanded.setOnClickListener {
                isExpanded = !isExpanded
                notifyDataSetChanged()
            }
        }
    }
}

 

 

run 사용2

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {
    holder.title.text = items[position].title + " $position"
    
    holder.run {
        nestedRV.apply {
            layoutManager = LinearLayoutManager(
                holder.nestedRV.context,
                LinearLayoutManager.VERTICAL,
                false
            )
            adapter = ChildAdapter(items[position].child)
            setRecycledViewPool(viewPool)
        }
        items[position].run {
            nestedRV.visibility = if(isExpanded) View.VISIBLE else View.GONE
            borderline.visibility = if(isExpanded) View.VISIBLE else View.GONE
            expanded.setOnClickListener {
                isExpanded = !isExpanded
                notifyDataSetChanged()
            }
        }
    }
}

 

 

with 사용 

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {
    holder.title.text = items[position].title + " $position"
    
    with(holder) {
        nestedRV.layoutManager = GridLayoutManager(
            nestedRV.context,
            5
        )
        nestedRV.adapter = ChildAdapter(items[position].child)
        nestedRV.setRecycledViewPool(viewPool)

        with(items[position]) {
            nestedRV.visibility = if(isExpanded) View.VISIBLE else View.GONE
            expanded.setOnClickListener {
                isExpanded = !isExpanded
                notifyDataSetChanged()
            }
        }
    }
}

 

Answer 1

해당 함수들을 scope  function라고 합니다. Scope funcrion은 편리하긴 하지만 남발하게 되면 문제가 가독성과 scope이 꼬일 수 있는 부작용이 있기 때문에 가능하면 중첩을 하지 않는 것이 좋습니다.그리고 apply, run, let, also는 제 각기 용도가 살짝씩 다르기 때문에 제일 적절한 함수를 사용해야 합니다.기본용도는 아래와 같습니다.

타입을 변환하여 리턴할 때: run, let
자기자신을 리턴할 때: apply, also
함수의 scope을 변경할 때: let, also

개인적으로는 님의 코드 중에서는 첫번째가 제일 읽기 편해 보이구요.  아래처럼 scope이 다른 부분은 함수를 분리해서 scope function을 그대로 사용하되, 중첩된 scope를 사용하지 않고도 처리할 수 있을 것 같습니다.

/*
   items의 display title 매핑. 어댑터 내부보다는 외부에서 데이터를 가져올 때 매핑하는 것이 어댑터가 데이터를 조작할 필요가 없으므로, 더 나오 보임.
   val listItems = originalItem.mapIndexed { position, item ->
       val newItem = NewItem()
       newItem.title = "${item.title} $position"
  }
  adapter.setItems(listItems)
*/

@SuppressLint("NotifyDataSetChanged")
override fun onBindViewHolder(holder: ParentViewHolder, position: Int) {    
    val item = items[position]
    holder.bind(items[position], ::onItemClicked)
}

private fun onItemClicked(item: Item) {
   item.isExpanded = !item.isExpanded
   notifyDataSetChanged() // ListAdapter를 사용하면 퍼포먼스도 더 나아지고 이 함수를 호출할 필요없음.
}

// ParentViewHolder
fun bind(item: Item, itemClicked: (Item) -> Unit) {
   bindParentView(item, itemClicked)
   bindNestedView(item)
}

private fun bindParentView(item: Item, itemClicked: (Item) -> Unit) {
     binding.apply {
        title.text = item.title
        borderline.isVisible = item.isExpanded
        expand.setOnClickListener {
            itemClicked(item)
        }
    }
}

private fun bindNestedView(item: Item) {
    binding.nestedRV.apply {
        layoutManager = GridLayoutManager(nestedRV.context, 5)
        adapter = ChildAdapter(items[position].child)
        setRecycledViewPool(viewPool)
        isVisible = item.isExpanded // isVisible은 KTx function
    }
}

 

Comments

감사합니다. 말씀하신 제 첫번째 코드라는게 apply 코드를 말씀하신거죠?
apply 경우에도 중첩 apply를 사용했는데 괜찮을까요

---

오리지널코드요.
괜찮고 말고는 코드가 정상 동작하는한 문제는 없겠죠. 포인트는 가독성, 특히 팀으로 일할 때, 와 중첩 스콥에 따른 부작용의 여부예요.