What happens if my RecyclerView adapter is reinitialized several times?-CodePudding

I have some recycler view code in a function that gets called several times as bluetooth devices are scanned. My code is working but I am wondering what unseen effects are occurring from having my recycler view initialization code in a function that gets repeated a lot? I eventually want to update the list rather than replace it via notifyDataSetChanged() but I am unsure how to do that with my current code structure. Any help would be appreciated!

@SuppressLint("MissingPermission", "NotifyDataSetChanged")
    fun displayDevices(
        scannedDevicesStrings: TreeSet<String>,
        deviceMap: HashMap<String, String>
    ) {
        
        val sortedDeviceMap = deviceMap.toSortedMap()

        Log.d(TAG, "displayDevices: ${sortedDeviceMap.entries}")

        // Set linear layout manager for the widget.
        val linearLayoutManager = LinearLayoutManager(applicationContext)
        binding.recyclerviewDevices.layoutManager = linearLayoutManager

        // Specify an adapter.
        listAdapter = CustomAdapter(scannedDevicesStrings.toList(), sortedDeviceMap, bluetoothManager)
        binding.recyclerviewDevices.adapter = listAdapter

        listAdapter.notifyDataSetChanged()

        // Notify the view to update when data is changed.
        if ( binding.recyclerviewDevices.isAttachedToWindow) {
            binding.progressBarCyclic.visibility = GONE
        }
    }

This code calls my CustomAdapter() class which looks like this:

class CustomAdapter(
    private val treeSet: List<String>,
    private var hashMap: SortedMap<String, String>,
    private val bluetoothManager: BluetoothManager
    ) : RecyclerView.Adapter<CustomAdapter.ViewHolder>() {

    class ViewHolder(view: View) : RecyclerView.ViewHolder(view) {
        val textView: TextView = view.findViewById(R.id.textview_list_item)
        val listLayout: FrameLayout = view.findViewById(R.id.item_layout)
        val context: Context = view.context
        val textView2: TextView = view.findViewById(R.id.textview_list_item_address)
    }

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
        val view = LayoutInflater.from(parent.context)
            .inflate(R.layout.text_device_row_item, parent, false)

        return ViewHolder(view)
    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {
        val deviceList = hashMap.keys.toList()
        val macAddressList = hashMap.values.toList()
        holder.textView.text = deviceList.elementAt(position)
        holder.textView2.text = macAddressList.elementAt(position)
        val selectedDeviceString = deviceList.elementAt(position).toString()
        val selectedDevice = bluetoothManager.adapter.getRemoteDevice(hashMap[selectedDeviceString])
        val sharedPreferences = holder.context.getSharedPreferences("mSharedPrefs", Context.MODE_PRIVATE) ?: return
            with(sharedPreferences.edit()) {
                putString("selectedDeviceString", selectedDevice.toString())
                apply()
            }
        holder.listLayout.setOnClickListener {
            val intent = Intent(holder.context, DeviceActivity::class.java)
            intent.putExtra("btDevice", selectedDevice)
            intent.putExtra("btDeviceName", selectedDeviceString)
            sharedPreferences.edit().putString("loadedFrom", "loadedFromCustomAdapter").apply()
            sharedPreferences.edit().putString("selectedDeviceName", selectedDeviceString).apply()
            sharedPreferences.edit().putString("selectedDeviceString", selectedDevice.toString()).apply()
            holder.context.startActivity(intent)
        }
    }

    override fun getItemCount() = treeSet.size
}

CodePudding user response：

Setting a new adapter makes the RecyclerView reinitialise itself, and it'll create all the ViewHolders again, etc. You'd want to avoid that really. This is generally how you'd make it update:

class CustomAdapter(
    private var data: List<Thing>
    ...
) {

    fun setData(data: List<Thing>) {
        // store the data and do any other setup work
        this.data = data
        // make sure the RecyclerView updates to show the new data
        notifyDataSetChanged()
    }

}

Then you just need to keep a reference to the adapter when you first create it in onCreate or whatever, and call theAdapter.setData(newData) whenever you get the new stuff. You're just setting up by creating an adapter to handle displaying your data in the list, and then you hand it data whenever it needs to update.

The actual "how things update" logic is in setData - it's the adapter's business how the adapter works internally, y'know? Right now it's the most basic notifyDataSetChanged() call, i.e. "refresh everything", but you could change that later - the outside world doesn't need to care about that though.

I noticed in onBindViewHolder you're doing this:

override fun onBindViewHolder(holder: ViewHolder, position: Int) {
    val deviceList = hashMap.keys.toList()
    val macAddressList = hashMap.values.toList()

That function runs every time a ViewHolder needs to display some new information (just before it scrolls onto the screen, basically) so you're creating a lot of lists whenever you scroll. Really you should do that once, when the data is set - derive your lists from the source and keep them around:

class CustomAdapter(
    initialData: List<Thing> // not a var now - see init block
    ...
) {

    // these might need to be lateinit since you're init-ing through a function
    private var data: List<Thing>
    private var deviceList: List<String>
    private var macAddressList: List<String>

    init {
        setData(initialData)
    }

    fun setData(data: List<Thing>) {
        // Now you're storing the data and deriving the other lists
        // You might not even need to store the 'data' object if you're not using it?
        this.data = data
        deviceList = data.keys.toList()
        macAddressList = data.values.toList()

        // make sure the RecyclerView updates to show the new data
        notifyDataSetChanged()
    }

}

So now setData takes some source data, derives the lists you need to use and stores those, and calls the update function. Because that setup has to be done, you need to call this function every time the source data is set - including when you first create the adapter. That's why the data parameter in the constructor isn't a var, it's just used in the initialisation, passed to the setData call.

Or alternatively, don't pass any data in the constructor at all - just create the adapter, and immediately call setData on it. Initialise the variables to emptyList() etc, and then you don't need to handle the "setup at construction time" case at all!

Just another couple of tips - I don't know what treeSet is for, but you're using its size in getItemCount. You shouldn't do that, it should usually reflect the size of the data set you're actually displaying, which is the contents of hashSet - when you get a position in onBindViewHolder, you're looking up an element in hashSet, not treeSet, so that should be your source for the number of items

The other thing is, all that stuff in onBindViewHolder... you're doing a lot of setup work that should really only happen when the item is actually clicked. Usually you'd set up the click listener once, in onCreateViewHolder, and when binding you'd set a field on the viewholder telling it which position it's currently displaying. If the click listener fires, then you can look up the current position in the data, create Intents, etc

Even if you don't move that into the VH, at least move the setup code into the onClickListener so it doesn't run every time a new item scrolls into view. That sharedPreferences bit is especially a problem - that gets overwritten every time a new item is bound (and they can be bound when they're still off-screen) so it probably isn't set to what you expect

CodePudding user response：

Setting the adapter multiple times should be avoided. Doing so causes its scroll position to be lost and reset to the top, and causes it to have to reinflate all of its views and ViewHolders. Instead, you should update the model your adapter points at and notifyDataSetChanged() on it (or better yet, use DiffUtil to update individual items).

CodePudding user response：

I finished updating my code and it works great! The data no longer jumps to the top when new data is added. Thought I would post the code for anyone who is interested.

Here is my adapter:

class CustomAdapter(
    private val bluetoothManager: BluetoothManager
    ) : RecyclerView.Adapter<CustomAdapter.ViewHolder>() {

    private var sortedMap = emptyMap<String, String>()
    private var deviceList = emptyList<String>()
    private var macAddressList = emptyList<String>()

    class ViewHolder(view: View) : RecyclerView.ViewHolder(view) {
        val textView: TextView = view.findViewById(R.id.textview_list_item)
        val listLayout: FrameLayout = view.findViewById(R.id.item_layout)
        val context: Context = view.context
        val textView2: TextView = view.findViewById(R.id.textview_list_item_address)
    }

    @SuppressLint("NotifyDataSetChanged")
    fun setData(sortedMap: SortedMap<String, String>) {
        this.sortedMap = sortedMap
        deviceList = sortedMap.keys.toList()
        macAddressList = sortedMap.values.toList()

        notifyDataSetChanged()
    }

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
        val view = LayoutInflater.from(parent.context)
            .inflate(R.layout.text_device_row_item, parent, false)

        return ViewHolder(view)
    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {

        holder.textView.text = deviceList.elementAt(position)
        holder.textView2.text = macAddressList.elementAt(position)

        holder.listLayout.setOnClickListener {
            val selectedDeviceString = deviceList.elementAt(position).toString()
            val selectedDevice = bluetoothManager.adapter.getRemoteDevice(sortedMap[selectedDeviceString])
            val sharedPreferences = holder.context.getSharedPreferences("mSharedPrefs", Context.MODE_PRIVATE) ?: return@setOnClickListener
            with(sharedPreferences.edit()) {
                putString("selectedDeviceString", selectedDevice.toString())
                apply()
            }

            val intent = Intent(holder.context, DeviceActivity::class.java)
            intent.putExtra("btDevice", selectedDevice)
            intent.putExtra("btDeviceName", selectedDeviceString)
            sharedPreferences.edit().putString("loadedFrom", "loadedFromCustomAdapter").apply()
            sharedPreferences.edit().putString("selectedDeviceName", selectedDeviceString).apply()
            sharedPreferences.edit().putString("selectedDeviceString", selectedDevice.toString()).apply()
            holder.context.startActivity(intent)
        }
    }

    override fun getItemCount() = sortedMap.size
}

And the activity onCreate() code:

// Specify an adapter.
listAdapter = CustomAdapter(bluetoothManager)
binding.recyclerviewDevices.adapter = listAdapter

And my function that updates the data:

@SuppressLint("MissingPermission", "NotifyDataSetChanged")
    fun displayDevices(
        deviceMap: HashMap<String, String>
    ) {
        
        val sortedDeviceMap = deviceMap.toSortedMap()
        listAdapter.setData(sortedDeviceMap)

        Log.d(TAG, "displayDevices: ${sortedDeviceMap.entries}")

        // Notify the view to update when data is changed.
        if ( binding.recyclerviewDevices.isAttachedToWindow) {
            binding.progressBarCyclic.visibility = GONE
        }
    }