Android RecyclerView holding rows of razzles

Question

I have a RecyclerView that works and everything, and this is how I coded it:

public class RazzleRecyclerViewAdapter extends RecyclerView.Adapter<RazzleRecyclerViewAdapter.RecyclerViewHolder> {
 private List<Razzle> mRazzleList;
 private Context mContext;
 private SelectRazzleRowListener mSelectRazzleRowListener;
 public RazzleRecyclerViewAdapter(
 Context context,
 List<Razzle> razzleList,
 SelectRazzleRowListener selectRazzleRowListener) {
 mContext = context;
 mRazzleList = razzleList;
 mSelectRazzleRowListener = selectRazzleRowListener;
 }
 @Override
 public RecyclerViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
 return new RecyclerViewHolder(LayoutInflater.from(parent.getContext())
 .inflate(R.layout.item_razzle_row, parent, false));
 }
 @Override
 public void onBindViewHolder(RecyclerViewHolder holder, int position) {
 holder.bindRow(mRazzleList.get(position), position);
 }
 @Override
 public int getItemCount() {
 return mRazzleList.size();
 }
 public class RecyclerViewHolder extends RecyclerView.ViewHolder {
 public int thisPosition;
 public Razzle mThisRazzle;
 public TextView razzleNameTextView;
 public RecyclerViewHolder(View itemView) {
 super(itemView);
 razzleNameTextView = (TextView) itemView.findViewById(R.id.item_razzle_row_textview_razzle_name);
 itemView.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View v) {
 mSelectRazzleRowListener.launchRazzleDetailsActivity(mThisRazzle, thisPosition);
 }
 });
 }
 public void bindRow(final Razzle razzle, int position) {
 mThisRazzle = razzle;
 thisPosition = position;
 razzleNameTextView.setText(razzle.getRazzleName());
 }
 }
}

How can I code this better? Can I make it clearer? Am I following good conventions? Can I restructure it so it's easier to work with? In particular, the ViewHolder stuff?

Answer 1

1. Remove mContext member. You're not using it anywhere.
2. Your code tells us that onClickListener will not change during life of Adapter object. Please make ViewHolder class static to not have implicit reference to Adapter and pass OnClickListener as a constructor parameter to ViewHolder.
3. ViewHolder class and it's bindRow method can be private.

Point 2 explanation :

You are passing SelectRazzleRowListener to Adapter class via constructor and don't provide a setter method to change that field during lifetime of Adapter object - so once adapter will be created with that listener he uses it always. And that's ok. But in that situation you can do a little tune to ViewHolder class. For now it's non-static inner class. By default if inner class is not static it contains implicit reference to parent class object (Adapter in this case). It may be risky in terms of memory leaks and it's good to be aware of that relationship (it's easy to test - try to create ViewHolder object somewhere outside adapter class in current state - without static - you won't be able to). It's also a reason why you can access Listener object from adapter inside ViewHolder class. In my opinion you should make ViewHolder class static (so public static class RecyclerViewHolder .... Then you need to pass SelectRazzleRowListener object to ViewHolder by defining a field in ViewHolder class and constructor parameter. Then Adapter will pass Selection listener to ViewHolder during onCreateViewHolder.

• user6846563: Can you please explain point 2 a bit more?
• Pawel Urban: @user6846563 I provided explanation for point 2. Let me know if it's sufficent enough for you ? :)