TintBrowser icon indicating copy to clipboard operation
TintBrowser copied to clipboard

Violation of "view holder" patttern

Open 00conan00 opened this issue 10 years ago • 1 comments

Dear developers,

I love TintBrowser, and recently I am writing a static code analysis tool to conduct performance analysis for Android apps. I found several violations of "view holder" patterns in TintBrowser's code.

Currently in TintBrowser, in some list adapters the getView() method works likes this

    public View getView(int position, View convertView, ViewGroup parent) {
        View v = super.getView(position, convertView, parent);

        TextView tv = (TextView) v.findViewById(android.R.id.text1);
        tv.setText(getItem(position).getTitle());

        return v;
    }

When the users scroll a list view, this method is going to build new views continuously instead of using the recycled "convertView". This wastes a lot of CPU cycles in "view inflation" and RAM in storing view objects. On low end devices, GC will kick in frequently because of RAM pressure. This will reduce the UI smoothness when a list has many items (even a list only has a few items, continuously building new views during scrolling is a waste of computational resource).

Google suggested a faster way for getView(), it works like this:

public View getView(int position, View convertView, ViewGroup parent) {
    ViewHolder holder;
    if(convertView == null){
        //we have no recycled views to use, then build new one

    } else {
        //use the recycled view to improve performance

    }
    return convertView;
}

I noticed that in the following adapters in the TintBrowser, the view holder pattern is correctly applied:

org/tint/ui/fragments/HistoryFragment$HistoryChildWrapper org/tint/ui/preferences/AddonsFragment$AddonsAdapter org/tint/ui/preferences/WebsitesSettingsFragment$SiteAdapter org/tint/ui/fragments/HistoryFragment$HistoryGroupWrapper

However, in the following adapters, the view holder pattern is not correctly implemented:

org/tint/model/UrlSuggestionCursorAdapter org/tint/ui/preferences/SslExceptionsFragment$SslExceptionAdapter org/tint/ui/activities/EditBookmarkActivity$FoldersAdapter org/tint/model/BookmarksAdapter

So I am curious :) Looking forward to your response. Thanks.

References: view holder pattern: http://lucasr.org/2012/04/05/performance-tips-for-androids-listview/ http://developer.android.com/training/improving-layouts/smooth-scrolling.html http://www.youtube.com/watch?v=wDBM6wVEO70

00conan00 avatar Mar 26 '14 12:03 00conan00

Your tool make false positives: calling super.getView in ArrayAdapter and SimpleCursorAdapter, which classes you quoted extend, does result in view recycling, see code for CursorAdapter which SimpleCursorAdapter is based on, and ArrayAdapter code.

ElementW avatar May 31 '14 12:05 ElementW