Why Is Adding an Onclicklistener Inside Onbindviewholder of a Recyclerview.Adapter Considered Bad Practice

Why is adding an OnClickListener inside onBindViewHolder of a RecyclerView.Adapter considered bad practice?

The reason it is better to handle your click logic inside the ViewHolder is because it allows for more explicit click listeners. As expressed in the Commonsware book:

Clickable widgets, like a RatingBar, in a ListView row had long been in conflict with click events on rows themselves. Getting rows that can be clicked, with row contents that can also be clicked, gets a bit tricky at times. With RecyclerView, you are in more explicit control over how this sort of thing gets handled… because you are the one setting up all of the on-click handling logic.

By using the ViewHolder model you can gain a lot of benefits for click handling in a RecyclerView than previously in the ListView. I wrote about this in a blog post comparing the differences - https://androidessence.com/recyclerview-vs-listview

As for why it is better in the ViewHolder instead of in onBindViewHolder(), that is because onBindViewHolder() is called for each and every item and setting the click listener is an unnecessary option to repeat when you can call it once in your ViewHolder constructor. Then, if your click responds depends on the position of the item clicked, you can simply call getAdapterPosition() from within the ViewHolder. Here is another answer I've given that demonstrates how you can use the OnClickListener from within your ViewHolder class.

Did i need to share an item from onBindViewHolder() to ViewHolder?

I don't know exactly what you're talking about but from what I understand, Your view holder can contain all the the UI and methods and logics, however I prefer the third method as it is more clear than the rest 2.

That being said please Look HERE AdaMc331 explains how do these methods work and how should you use them.

Is it bad practice to pass a fragment as argument to RecyclerView.Adapter? To help onClick

No, there's no problem with doing this. Your Adapter's lifecycle is tied to your Fragment's lifecycle, so you're not risking any memory leaks or anything like that by doing this.

You might want to do this via an interface that defines only that method in order to enforce the API between the two for separation-of-concerns reasons, but this is also fine.

RecyclerView item onClickListener doesn't work on a first click, but works on a second one

I believe your problem is in this line

movieConstraint.setOnClickListener(this);

You shouldn't set it to ConstraintLayout, change the above line to this

itemView.setOnClickListener(this);

And there is nothing wrong in setting your onClickListener in a ViewHolder constructor.

EDIT:

In order to get the position in onClick you can use getAdapterPosition() method.

RecyclerView element OnClickListener does not react to clicks

The problem is that I named the variable for MyViewHolder "view", just like the first variable declared in onCreateView. Renaming the variable to "item" solved the problem:

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
    val view = inflater.inflate(R.layout.file_explorer, container, false)

    class MyViewHolder(val item: View): RecyclerView.ViewHolder(item) {
        init {
            item.setOnClickListener {
                Log.d("path", files[adapterPosition].path)
                Log.d("pos", adapterPosition.toString())
                goto(files[adapterPosition].path)
            }
        }
    }

    val recyclerView = view.findViewById<RecyclerView>(R.id.files)
    recyclerView.addItemDecoration(DividerItemDecoration(recyclerView.context, DividerItemDecoration.VERTICAL))
    recyclerView.adapter = object: RecyclerView.Adapter<MyViewHolder>() {
        override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): MyViewHolder {
            val item = LayoutInflater.from(parent.context).inflate(R.layout.item, parent, false)
            return MyViewHolder(item)
        }

        override fun onBindViewHolder(holder: MyViewHolder, position: Int) {
            holder.item.findViewById<TextView>(R.id.text).text = files[position].name
        }

        override fun getItemCount() = files.size
    }

    return view
}

The mechanics of this closure resolution elude me though and I've created a new question.

RecyclerView onClick

As the API's have radically changed, It wouldn't surprise me if you were to create an OnClickListener for each item. It isn't that much of a hassle though. In your implementation of RecyclerView.Adapter<MyViewHolder>, you should have:

private final OnClickListener mOnClickListener = new MyOnClickListener();

@Override
public MyViewHolder onCreateViewHolder(final ViewGroup parent, final int viewType) {
    View view = LayoutInflater.from(mContext).inflate(R.layout.myview, parent, false);
    view.setOnClickListener(mOnClickListener);
    return new MyViewHolder(view);
}

The onClick method:

@Override
public void onClick(final View view) {
    int itemPosition = mRecyclerView.getChildLayoutPosition(view);
    String item = mList.get(itemPosition);
    Toast.makeText(mContext, item, Toast.LENGTH_LONG).show();
}

Is passing an object through constructors, to multiple fragments a bad practice?

My question is, is this a bad practice in Android / OOP in general?

Yes.

Or is there a better way of doing what I'm trying to achieve?

Probably.

What that is is hard to say since it's not fully clear what you're trying to achieve and how this code is structured.

But I would advise you check out this documentation on sharing data between fragments. With that in mind, consider keeping this OrderItem object in one place - the ViewModel - then accessing the one shared view model from each Fragment or Activity that needs to work with / on the order item.

RecyclerView onClick does not work

I'm sure it will be easier for you to set the click listener inside the onBindViewHolder.
containerView - get this view from your viewholder

    @Override
    public void onBindViewHolder(TaskViewHolder holder, int position) {
            containerView.setOnClickListener(new DefaultInterfaceImplUtils.ClickListener() {
                @Override
                public void onViewClicked(View v) {
                    AlertDialog.Builder builder = new AlertDialog.Builder(MainActivity.this);
                    builder.setView(getLayoutInflater().inflate(R.layout.edit_dialog, null));
                    builder.setTitle("Edit Item");
                    builder.setNeutralButton("TEST", new DialogInterface.OnClickListener() {
                        @Override
                        public void onClick(DialogInterface dialogInterface, int i) {
                            Log.d("TEST", "Test button was clicked in AlertDialog");
                        }
                    });
                    builder.setCancelable(true);
                    builder.show();
                }
            }
        });
    }

---

Related Topics