In a list adapter is there a reason i should check for existing view?

Question

I have an expandable list adapter. In it I'm inflating the view like so

if (view == null) {

    view = LayoutInflater.from(mContext).inflate(R.layout.trigger_list_item, null);

    holder = new ViewHolder();
    holder.mTrigger = trigger;
    holder.triggerName = view.findViewById(R.id.triggerName);
    holder.upButton = view.findViewById(R.id.mTriggerButtonUp);
    holder.downButton = view.findViewById(R.id.mTriggerButtonDown);
    holder.total = view.findViewById(R.id.triggerCounter);

    view.setTag(holder);
}
else {
    holder = (ViewHolder) view.getTag();
}

I was having an issue where a change i made to one view would sometimes, though not always, effect others. When I took out the check for view and null, it worked, leaving this.

view = LayoutInflater.from(mContext).inflate(R.layout.trigger_list_item, null);

holder = new ViewHolder();
holder.mTrigger = trigger;
holder.triggerName = view.findViewById(R.id.triggerName);
holder.upButton = view.findViewById(R.id.mTriggerButtonUp);
holder.downButton = view.findViewById(R.id.mTriggerButtonDown);
holder.total = view.findViewById(R.id.triggerCounter);

view.setTag(holder);

This appears to work, but I had always learned to check for the view already existing with if (view == null). Is there a reason I shouldn't accept this fix? Are there memory leaks or anything associated with this?

Answer 1

I was having an issue where a change i made to one view would sometimes, though not always, effect others.

This usually happens when you only modify a part of the view sometimes, instead of always. For instance, you might have

if (position % 3 == 0) {
    someView.setBackgroundColor(0xff0000ff);
}

In cases where your view is recycled and passed to getView() later on (as the convertView param), it will still have a blue background. You should instead write your code to always set the background color:

if (position % 3 == 0) {
    someView.setBackgroundColor(0xff0000ff);
} else {
    someView.setBackgroundColor(0xffffffff);
}

I had always learned to check for the view already existing with if (view == null). Is there a reason I shouldn't accept this fix?

Yes: inflating a view is expensive and time-consuming. If you want to make sure your app doesn't drop any frames when the user flings your list, you should leverage the recycling capability getView() provides. It's not like inflating a view is agonizingly slow, but if you inflate a new view every time (and therefore also use lots of findViewById() calls every time), you will definitely notice less-smooth scrolling than if you follow the view holder pattern.