Showing posts with label Gotcha. Show all posts
Showing posts with label Gotcha. Show all posts

Thursday, May 22, 2014

Clicking unclickable list items

One of the UI patterns that improve lists usability is dividing items into sections. The section might be the first letter of the main text on the list item, date formatted and rounded in a specific way or whatever makes sense for your data.

From the technical point of view you can either add the header view to every list item and show and hide them as needed or create the separate view for header and regular list item and register multiple view types in your Adapter. Both options were described in details by +Cyril Mottier in excellent ListView Tips & Tricks #2: Sectioning Your ListView blog post.

If you choose the second approach, you'll have to decide what to return from your Adapter's getItem and getItemId methods for items representing sections. If your sections are not supposed to be clickable, you might implement your Adapter like this:
@Override
public Object getItem(int position) {
  return getItemViewType(position) == TYPE_ITEM
      ? mItems[getCursorPosition(position)]
      : null;
}

@Override
public long getItemId(int position) {
  return getItemViewType(position) == TYPE_ITEM
      ? getCursorPosition(position)
      : 0;
}

@Override
public boolean areAllItemsEnabled() {
  return false;
}

@Override
public boolean isEnabled(int position) {
  return getItemViewType(position) == TYPE_ITEM;
}
And your onListItemClickListener like this:
@Override
public void onListItemClick(ListView l, View v, int position, long id) {
  super.onListItemClick(l, v, position, id);

  // dummy action which uses Object returned from getItem(position)
  Log.d("DMFM", getListAdapter().getItem(position).toString());
}
If you do so, the Android has a nasty surprise for you:
java.lang.NullPointerException
    at org.chalup.dialmformonkey.app.MainFragment.onListItemClick(MainFragment.java:38)
    at android.app.ListFragment$2.onItemClick(ListFragment.java:160)
    at android.widget.AdapterView.performItemClick(AdapterView.java:298)
    at android.widget.AbsListView.performItemClick(AbsListView.java:1100)
    at android.widget.AbsListView$PerformClick.run(AbsListView.java:2749)
    at android.widget.AbsListView$1.run(AbsListView.java:3423)
    at android.os.Handler.handleCallback(Handler.java:725)
    ...
The only way this can happen is getting null from Adapter.getItem(), but this method will be called only for disabled items, right?
@Override
public void onListItemClick(ListView l, View v, int position, long id) {
  super.onListItemClick(l, v, position, id);

  Log.d("DMFM", "Clicked on item " + position + " which is " +
        (getListAdapter().isEnabled(position) 
            ? "enabled"
            : "disabled")
  );

  // dummy action which uses Object returned from getItem(position)
  Log.d("DMFM", getListAdapter().getItem(position).toString());
}
Wrong:
D/DMFM﹕ Clicked on item 4 which is enabled
D/DMFM﹕ Abondance
D/DMFM﹕ Clicked on item 4 which is enabled
D/DMFM﹕ Abondance
D/DMFM﹕ Clicked on item 31 which is enabled
D/DMFM﹕ Aragon
D/DMFM﹕ Clicked on item 31 which is enabled
D/DMFM﹕ Aragon
D/dalvikvm﹕ GC_CONCURRENT freed 138K, 3% free 8825K/9016K, paused 0ms+0ms, total 3ms
D/DMFM﹕ Clicked on item 28 which is disabled
It's very difficult to reproduce this error manually, especially if tapping the list item does something more than writing to logcat, but I investigated this issue, because the stack traces above appeared in crash reports on Google Analytics, so several people managed to click exactly wrong area at the wrong time.

I didn't investigate the issue thoroughly, but it seems there must be some disparity between checking the isEnabled method and getting the item. If I ever dive into ListView code, I'll definitely write about it. If you want to reproduce or investigate the issue yourself, compile this project and run the monkey:
$ adb shell monkey -p org.chalup.dialmformonkey.app -v 500
So what can we do? First option is checking the Adapter.isEnabled() in your onListItemClick listener, which is yet another kind of boilerplate you have to add to your Android code, but it's super easy to add. The other option is going with the first sectioning approach, i.e. putting section as a part of the clickable list item, but it might not work for your use case (for example adapter with multiple item types).

Saturday, January 25, 2014

Offline mode in Android apps, part 3 - old db schemas

The first post in this series explained the first consequence on implementing the offline mode - performing the data migrations. In second part I showed a workaround for the rudimentary SQLite's ALTER TABLE syntax. If you have checked the link to MigrationHelper class I mentioned, you migth have noticed that it's just a tiny part of a larger library, which allows you to define database schemas. Note the plural "schemas": the whole point of this library is defining both current schema and the schemas for the older versions of your app. This post explains why do you have to do this.

Let's say in the first version you have the following data structure:
public static class User {
  public long id;
  public String firstName;
  public String lastName;
  public String email;
}
And the table definition for this table in your SQLiteOpenHelper looks like this:
private static final String CREATE_TABLE_USERS = "CREATE TABLE " +
    TABLE_USERS +
    " ( " +
    ID + " INTEGER PRIMARY KEY AUTOINCREMENT " + ", " +
    FIRST_NAME + " TEXT " + ", " +
    LAST_NAME + " TEXT " + ", " +
    EMAIL + " TEXT " +
    " ) ";
In the next version you decide to keep only the first name in a single field, so you change your data structure accordingly and perform the data migration. In the snippet below I used the MigrationHelper, but you might have as well performed the migration by hand:
private static final String CREATE_TABLE_USERS = "CREATE TABLE " +
    TABLE_USERS +
    " ( " +
    ID + " INTEGER PRIMARY KEY AUTOINCREMENT " + ", " +
    NAME + " TEXT " + ", " +
    EMAIL + " TEXT " +
    " ) ";

@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
  MigrationsHelper helper = new MigrationsHelper();
  if (oldVersion < 2) {
    helper.performMigrations(db, 
        TableMigration.of(TABLE_USERS)
            .to(CREATE_TABLE_USERS)
            .withMapping(NAME, FIRST_NAME)
            .build()
    );
  }
}
Then you decide that the email field should be mandatory, so you change the schema and migrate the data again:
private static final String CREATE_TABLE_USERS = "CREATE TABLE " +
    TABLE_USERS +
    " ( " +
    ID + " INTEGER PRIMARY KEY AUTOINCREMENT " + ", " +
    NAME + " TEXT " + ", " +
    EMAIL + " TEXT NOT NULL" +
    " ) ";

@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
  MigrationsHelper helper = new MigrationsHelper();
  if (oldVersion < 2) {
    helper.performMigrations(db,
        TableMigration.of(TABLE_USERS)
            .to(CREATE_TABLE_USERS)
            .withMapping(NAME, FIRST_NAME)
            .build()
    );
  }
  if (oldVersion < 3) {
    db.execSQL("DELETE FROM " + TABLE_USERS + " WHERE " + EMAIL + " IS NULL");
    helper.performMigrations(db,
        TableMigration.of(TABLE_USERS)
            .to(CREATE_TABLE_USERS)
            .build()
    );
  }
}
The code looks fine, but you have just broken migrations from v1 to v3. If there is an user with a null email field, the app will crash in line 13 above. But why, shouldn't the email field in v2 schema be nullable? It should, but this migration uses the constant containing the latest schema definition with different column constraint.

The worst thing about this kind of bugs is that it might slip through your tests, because the crash happens only if you have a specific data before the application update.

You migth be tempted to define separate migrations from every old version to the latest one (in our case migrations from v1 to v3 and from v2 to v3) and always execute only single migration, but this workaround doesn't scale. For each new migration you'd have to check and potentially update every existing migration. When you publish the app twice a month, this quickly becomes a huge problem.

The other solution is to make every migration completely independent from the others, and execute them sequentially. This way, when you define a new migration, you don't have to worry about the previous ones. This means that when you upgrade from v1 to v3, you first upgrade from v1 to v2 and then from v2 to v2 and after the first step the database should be in the same state it were, when the v2 was the latest version. In other words, you have to keep an old database schemas.

As usual there are multiple ways to do this. You can copy the schema definition to another constant and append "ver#" suffix, but it means there will be a lot of duplicated code (although this code should never, ever change, so it's not as bad as the regular case of copypaste). The other way is to keep the initial database state and all the schema updates. The issue here is that you don't have a place in your code with current schema definition. The opposite solution is to keep the current schema and the list of downgrades. Sounds counterintuitive? Don't worry, that's because it *is* counterintuitive.

In android-schema-utils I've chosen the third approach, because in the long run it processes less data than the upgrades solution - in case of upgrade from vN-1 to vN it has to generate only 1 additional schema instead of N-1 schemas. I'm still not sure if the code wouldn't be clearer had I went with duplicated schema definitions approach, but the current approach, once you get used to it, works fine. The schema and migrations for our example would look like this:
private static final MigrationsHelper MIGRATIONS_HELPER = new MigrationsHelper();
private static final Schemas SCHEMAS = Schemas.Builder
    .currentSchema(3,
        new TableDefinition(TABLE_USERS,
            new AddColumn(ID, "INTEGER PRIMARY KEY AUTOINCREMENT"),
            new AddColumn(NAME, "TEXT"),
            new AddColumn(EMAIL, "TEXT NOT NULL")
        )
    )
    .upgradeTo(3,
        new SimpleMigration() {
          @Override
          public void apply(SQLiteDatabase db, Schema schema) {
            db.execSQL("DELETE FROM " + TABLE_USERS + " WHERE " + EMAIL + " IS NULL");
          }
        },
        auto()
    )
    .downgradeTo(2,
        new TableDowngrade(TABLE_USERS, new AddColumn(EMAIL, "TEXT"))
    )
    .upgradeTo(2,
        SimpleTableMigration.of(TABLE_USERS)
            .withMapping(NAME, FIRST_NAME)
            .using(MIGRATIONS_HELPER)
        )
    .downgradeTo(1,
        new TableDowngrade(TABLE_USERS,
            new AddColumn(FIRST_NAME, "TEXT"),
            new AddColumn(LAST_NAME, "TEXT"),
            new DropColumn(EMAIL)
        )
    )
    .build();
There are other benefits of keeping the old schemas in a more reasonable format than raw strings. Most of the schema migrations can be deducted from comparing subsequent schema versions, so you don't have to do it yourself. For example in migration from v2 to v3 I didn't have to specify that I want to migrate the Users table - the auto() migration automatically handles it. If the auto() is the only migration for a given upgrade, you can skip the whole upgradeTo() block. In our case that covered about 50% migrations, but YMMV.

If you go this way, your onUpgrade method, which usually is the most complex part of SQLiteOpenHelper, can be reduced to this:
@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
  SCHEMAS.upgrade(oldVersion, mContext, db);
}
This part concludes the "offline mode" series. Here's the short recap:

  • If you don't want to compromise on UX, your application should work regardless whether the user is connected to internet or not.
  • In this case the user may end up in a situation when the application is upgraded, but not all data is synced with the server yet. You *do not* want to lose your users' data. You'll have to migrate them.
  • If you migrate your data, you should keep the migrations separate from one another, because otherwise maintaining them becomes a nightmare.
  • The best way to do this that I know of, is keeping the old schemas and always performing all migrations sequentially. To make things simpler, I recommend the android-schema-utils library.

Sunday, January 12, 2014

Android SQLiteDatabase gotcha

In my previous post I mentioned a nasty SQLiteDatabase gotcha and recommended using the MigrationHelper utility I wrote. If you have checked this class's sources, you might have noticed a weird code. Before getting the list of columns the table is renamed to the temporary name and then renamed back:
final String tempTable = "tmp_" + tempTableIndex++;
db.execSQL("ALTER TABLE " + migration.tableName + " RENAME TO " + tempTable);
ImmutableSet<String> oldColumns = getColumns(db, tempTable);

db.execSQL(migration.createTableStatement);
final String tempNewTable = "tmp_" + tempTableIndex++;
db.execSQL("ALTER TABLE " + migration.tableName + " RENAME TO " + tempNewTable);
ImmutableSet<String> newColumns = getColumns(db, tempNewTable);

db.execSQL("ALTER TABLE " + tempNewTable + " RENAME TO " + migration.tableName);

private static ImmutableSet<String> getColumns(SQLiteDatabase db, String table) {
Cursor cursor = db.query(table, null, null, null, null, null, null, "0");
if (cursor != null) {
try {
return ImmutableSet.copyOf(cursor.getColumnNames());
} finally {
cursor.close();
}
}
return ImmutableSet.of();
}
Initially the MigrationHelper's code looked like this:
static final String TEMP_TABLE = "tmp";
db.execSQL("ALTER TABLE " + migration.tableName + " RENAME TO " + TEMP_TABLE);
ImmutableSet<String> oldColumns = getColumns(db, TEMP_TABLE);

db.execSQL(migration.createTableStatement);
ImmutableSet<String> newColumns = getColumns(db, migration.tableName);
It worked for a single migration, but didn't work for multiple migrations - the helper method for getting the column set always returned the columns of first table. Since the query was always the same, I suspected the results are cached somewhere. To verify this hypothesis I added to the temporary table name an index incremented with every migration. It worked, but then I realized I need to do the same for getting the columns of the new schema - otherwise the helper wouldn't work if the same table were migrated twice. This way the weird code was born.

But the same thing could happen outside of MigrationHelper operations, for example if you need to iterate through rows of the same table in two different migrations:
@Override
public void onUpgrade(final SQLiteDatabase db, int oldVersion, int newVersion) {
  if (oldVersion <= 1500) {
    Cursor c = db.query("some_table", /* null, null, null... */);
    // use Cursor c
  }

  // other migrations, including ones that change the some_table table's columns

  if (oldVersion <= 2900) {
    Cursor c = db.query("some_table", /* null, null, null... */);
    // try to use Cursor c and crash terribly
  }
}
So I checked the AOSP code for the suspected cache to see how the entries can be evicted or if the cache can be disabled. There are no methods for this, so you can't do it with straightforward call, but maybe you can exploit the implementation details?

On ICS the cache is implemented as LruCache, so theoretically you could evict old entries by filling the cache with new ones, but there is one hiccup - you don't know the cache size, so you'd always have to go with MAX_SQL_CACHE_SIZE.

Before ICS you couldn't do even that - the implementation of this "cache" is just a fixed size buffer for SQLiteStatements. Once that buffer is full, no more statements are cached. This also has one more consequence - your app might work much slower on Android 2.x after upgrade from old version than after fresh install, because the db cache will be filled with queries used in migrations.

Fortunately the keys of this cache are raw SQL strings, so we can disable cache for migration queries by adding "WHERE n==n" clause with n incremented for every query (note that you musn't pass n as a bound parameter - the whole point of adding this selection is to make the queries different and force  SQLiteDatabase to compile another statement).

The question you should ask yourself is why do I have to know and care about all this. Isn't SQLite smart enough to see that I'm trying to access the database using prepared statement compiled against old schema? It turns out the SQLite detects this issues and raises SQLITE_SCHEMA error (commented with "The database schema changed"), but Android's SQLiteDatabase wrapper drops this error and happily uses the old, invalid statements. Bad Android.

Tuesday, December 3, 2013

SQLite views gotcha

tl;dr: don't left join on view, or you gonna have a bad time.

I have investigated a performance issue of the db in Android app today. The symptoms looked like a classic case of the missing index: the performance degraded with adding more data to certain tables. However, the quick check of sqlite_master table and looking at some EXPLAIN QUERY PLAN queries indicated that everything is properly indexed (which is not very surprising, given that we use android-autoindexer).

I started dumping the explain query plans for every query and it turned out that some queries perform multiple table scans instead of single scan of main table + indexed searches for joined tables. It means that the indices were in place, but they weren't used.

The common denominator of these queries was joining with a view. Here's the simplest schema which demonstrates the issue:

sqlite> create table x (id integer);
sqlite> create table y (id integer, x_id integer);

sqlite> explain query plan select * from x left join y on x.id = x_id;
selectid    order       from        detail
----------  ----------  ----------  ----------------------------------------------------------------
0           0           0           SCAN TABLE x (~1000000 rows)
0           1           1           SEARCH TABLE y USING AUTOMATIC COVERING INDEX (x_id=?) (~7 rows)

sqlite> create view yyy as select * from y;

sqlite> explain query plan select * from x left join yyy on x.id = x_id;
selectid    order       from        detail
----------  ----------  ----------  -------------------------------------------------------------------
1           0           0           SCAN TABLE y (~1000000 rows)
0           0           0           SCAN TABLE x (~1000000 rows)
0           1           1           SEARCH SUBQUERY 1 USING AUTOMATIC COVERING INDEX (x_id=?) (~7 rows)

Of course this behaviour is documented in the SQLite Query Planner overview (point 3 of the Subquery flattening paragraph), and I even remember reading this docs few times, but I guess something like this has to bite me in the ass before I memorize it.

Everything works fine if you copypaste the views selection in place of the joined view, which makes me a sad panda, because I wish SQLite could do this for me. On the other hand it's a very simple workaround for this issue, and, with a right library, the code might even be manageable.

Wednesday, June 26, 2013

Guava and minSdkVersion

A while ago I wrote about pre-dexing feature introduced at the end of 2012, which facilitates using large Java libraries like Guava for developing Android apps. Few months later, I'm still discovering stuff in Guava that makes my life easier (BTW: I still owe you a blog post with a list of Guava goodies). But this week, for a change, I've managed to make my life harder with Guava.

I wanted to include the javadocs and source jars for Guava, and when I opened maven central I saw the new version and decided to upgrade from 13.0.1 to 14.0.1. Everything went smoothly except for the minor Proguard hiccup: you have to include the jar with the @Inject annotation. At least it went smoothly on the first few phones I've tested our app on, but on some ancient crap with Android 2.2 the app crashed with NoClassDefFoundError.

The usual suspect in this case is, of course, Proguard. I've also suspected the issue similar to the libphonenumber crash I wrote about in March. When both leads turned out to be a dead end, I decided to run the debug build and to my surprise it crashed as well. And there was a logcat message which pinpointed the issue: the ImmutableSet in Guava 14.0.1 depends somehow on NavigableSet interface, which is available from API level 9. Sad as I was, I downgraded the Guava back to 13.0.1 and everything started to work again.

So what have I learned today?

  • Upgrading the libraries for the sake of upgrading is bad (m'kay).
  • Before you start wrestling with Proguard, test the debug build.
  • Android 2.2 doesn't support all Java 1.6 classes.


The scary thing is, the similar thing might happen again if some other class in Guava depends on APIs unavailable on older versions of Android. Sounds like a good idea for a weekend hack project: some way to mark or check the minSdkVersion needed to use a given class or method.

Thursday, June 6, 2013

SQLite type affinity strikes back

About a year ago I have wrote about a certain SQLite gotcha on Android. tl;dr: in some cases when you create a view with unions, SQLite cannot determine a type of the column, and since Android binds all selection arguments as strings, SQLite ends up comparing X with "X", concludes those are not the same thing and returns fewer rows than you'd expect.

Recently the same problem reared it's ugly head. It turns out that it's very easy to create in a view a column with undefined type. It might happen in case of joins, using aggregation functions, subqueries, etc. pretty much anything more fancy than simple select. Therefore I recommend checking the columns type using the pragma table_info(table) command for every view:
sqlite> .head on
sqlite> .mode column
sqlite> pragma table_info (v);

cid         name        type        notnull     dflt_value  pk
----------  ----------  ----------  ----------  ----------  ----------
0           test                    0                       0

If the type of a column is undefined and you need to use this column in your selection arguments, you should add the UNION with an empty row with well defined column types:
sqlite> CREATE TABLE types (i INTEGER, t TEXT);
sqlite> CREATE VIEW vfix AS SELECT i AS test FROM types WHERE 1=0 UNION SELECT * FROM v;
sqlite> pragma table_info (vfix);

cid         name        type        notnull     dflt_value  pk
----------  ----------  ----------  ----------  ----------  ----------
0           test        INTEGER     0                       0

Wednesday, April 24, 2013

Android gotcha: CursorAdapter constructors

I just spend few hours analyzing and fixing a memory leak in Android application. With every orientation change the full Context, including the whole Activity was leaked. Long story short, the problem was caused by misuse of CursorAdapter: in subclass constructor we called CursorAdapter(context, null, false) instead of CursorAdapter(context, null, 0).

The difference is quite subtle. If you use the second constructor, you have to take care of handling content updates yourself. If you use the first constructor, the CursorAdapter will register an additional ContentObserver for you, but you need to manually reset the Cursor.

The funny thing is, this behavior is described in javadocs, but the documentation is spread between the constructor and FLAG_REGISTER_CONTENT_OBSERVER flag documentation. The second part contains most crucial information: you don't need to use this flag when you intend to use your adapter with CursorLoader.

If for some reason you want to use the adapter without CursorLoader, you should use the CursorAdapter(context, null, false) constructor, and call swapCursor(null) when leaving the Activity or Fragment.

Saturday, March 23, 2013

libphonenumber crash on Android 3.2

Few days ago I saw the most peculiar crash on a tablet with Android 3.2:

java.lang.NoSuchFieldError: com.google.i18n.phonenumbers.PhoneNumberUtil$PhoneNumberFormat.RFC3966
Of course there is such field in this class, it works on every other Android hardware I have access to. This was a debug build, so it couldn't have been the Proguard issue. The other functionality from the com.google.i18n.phonenumbers package worked fine, the issue only appeared if I wanted to format a phone number using this specific format.

Long story short, it turns out that some old version of libphonenumber, which doesn't support this particular phone number format, is included in the Android build on my 3.2 device. You can verify such thing by calling:
Class.forName("com.google.i18n.phonenumbers.PhoneNumberUtil");
inside a project without any libs - on this single 3.2 device it will return the valid Class object, on every other device I tried it throws ClassNotFoundException.

I started to wonder if any other libraries are affected, so I picked up some class names from the most popular libraries (as reported by AppBrain) and it seems there might be a similar issue with Apache Commons Codec jar. Fortunately there are no issues with stuff like Guava, GSON or support lib.

What's the workaround for this issue? Fork the library and change the package name.

Friday, December 28, 2012

UnknownHostException gotcha

Several days ago I was preparing a simple app for the programming contest organized by Future Simple during KrakDroid conference. It consisted of a public ContentProvider, SyncAdapter and an Activity. The contest participants had to write an app which retrieves the input data from ContentProvider, solve a very simple algorithmic problem and save the output back in the ContentProvider. This would trigger the sync and the SyncAdapter would upload the data to our backend. The Activity showed the contest rules and the list of submissions.

As you can see it's nothing fancy and I hacked everything together rather quickly, but when I tried to send the first submission I got the UnknownHostException from AbstractHttpClient.execute. Had I googled the issue first, I'd solve this issue in one minute. The problem is, my internet connection was flaky and the backend I tried to connect to was set up couple hours earlier, so the UnknownHostException seemed like a quite probable error.

If you haven't click the Google link above, here's the solution: this exception is a way in which Android tells you that your app is missing the INTERNET permission.

Wednesday, September 19, 2012

Android heterogeneous Adapters gotcha

Unless you were writing your Android apps under some kind of digital rock, you heard about Mark Murphy a.k.a. commonsguy. Today I'd like to write about a gotcha related to heterogeneous Adapters in general, which recently bit me in the rear when I used (misused?) one of Mark's Android components - MergeAdapter.

As you can read on the project's site, "MergeAdapter accepts a mix of Adapters and Views and presents them as one contiguous whole to whatever ListView it is poured into". This means of course that this is a heterogeneous adapter, i.e. the one which returns integer > 1 from getViewTypeCount(). The implementation of this method is pretty straightforward - it just iterates through the list of adapters it consists of and returns the sum of getViewTypeCount()s :
@Override
public int getViewTypeCount() {
  int total=0;

  for (PieceState piece : pieces.getRawPieces()) {
    total+=piece.adapter.getViewTypeCount();
  }

  return(Math.max(total, 1)); // needed for
                              // setListAdapter() before
                              // content add'
}
Everything is fine and dandy if you use the code like this:
@Override
public void onCreate(Bundle icicle) {
  super.onCreate(icicle);
  setContentView(R.layout.main);

  MergeAdapter adapter = new MergeAdapter();
  adapter.addView(someView);
  adapter.addAdapter(someAdapter);

  setListAdapter(adapter);
}
But sometimes you might want to attach the MergeAdapter to ListView and add fill it later (the real scenario for this case is adding stuff in onLoadFinished callback, I'm using contrived example for sake of simplicity):
@Override
public void onCreate(Bundle icicle) {
  super.onCreate(icicle);
  setContentView(R.layout.main);

  MergeAdapter adapter = new MergeAdapter();
  adapter.addAdapter(someAdapter);

  setListAdapter(adapter);

  adapter.addAdapter(someOtherAdapter);
}
This code will work as long as the adapter's contents fit on one screen, but if you start scrolling the list and the item recycling kicks in your app will crash with ClassCastException from your adapters' getView(). If by some chance you use the same IDs for the Views of the same type the app won't crash, but your items probably won't look exactly as they should. Either way, you won't be happy.

The root cause is the undocumented fact that the getViewTypeCount() is called only once after attaching it with ListView.setAdapter(). In the example above, the MergeAdapter contains only one item type when the setAdapter() is called, getViewTypeCount() will return 1, and adding the second adapter with another item type won't change this.

Why doesn't this crash right away? The ListView will call getView() in correct adapters, but then it will try to reuse items created by one adapter for items in the second adapter, because it assumes there is only one view type (getViewTypeCount() returned 1).

So what's the lesson here? Do not change the MergeAdapter in loader callbacks, either construct it before setAdapter() call (for example add empty CursorAdapters and call CursorAdapter.changeCursor() later) or postpone the setAdapter() call until you load all the parts. The more general rule is that you may not calculate the number of item types from the actual data, for example the following code won't work:
public class MyCursorAdapter extends CursorAdapter {
  // Implementation of bindView, newView, etc. skipped

  private int mCalculatedItemTypeCount;

  @Override
  public int getViewTypeCount() {
    return Math.max(mCalculatedItemTypeCount, 1);
  }

  @Override
  public void changeCursor(Cursor cursor) {
    mCalculatedItemTypeCount = /* some calculations */;
    super.changeCursor(cursor);
  }
}

Wednesday, September 5, 2012

Android SharedPreferences gotcha

I have another gotcha for you. Can you tell what's wrong with the following code?

public class MyFragment extends Fragment implements OnSharedPreferenceChangeListener {
  private TextView mInfo;
  private SharedPreferences mPreferences;

  public static final String INFO_SP_KEY = "info";
  
  @Override
  public View onCreateView(LayoutInflater inflater, ViewGroup container,
      Bundle savedInstanceState) {
    return inflater.inflate(R.layout.my_fragment, container, false);
  }

  @Override
  public void onViewCreated(View view, Bundle savedInstanceState) {
    super.onViewCreated(view, savedInstanceState);
    mInfo = (TextView) view.findViewById(R.id.info);
  }
  
  @Override
  public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key) {
    if (key.equals(INFO_SP_KEY)) {
      updateInfo();
    }
  }

  @Override
  public void onActivityCreated(Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);
    mPreferences = PreferenceManager.getDefaultSharedPreferences(getActivity());
  }

  @Override
  public void onPause() {
    super.onPause();
    mPreferences.unregisterOnSharedPreferenceChangeListener(this);
  }

  @Override
  public void onResume() {
    super.onResume();
    mPreferences.registerOnSharedPreferenceChangeListener(this);
    updateInfo();
  }

  protected void updateInfo() {
    mInfo.setText(getString(R.string.info_text, mPreferences.getInt(INFO_SP_KEY, 0)));
  }
}
At first glance everything looks fine and in most cases it will work fine as well. However, if you a) set android:minSdkVersion to 8 or lower and b) change the shared preference from another thread (IntentService, SyncAdapter, etc.), you'll get the following crash:

09-05 07:16:58.993: E/AndroidRuntime(403): android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.ViewRoot.checkThread(ViewRoot.java:2802)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.ViewRoot.requestLayout(ViewRoot.java:594)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.checkForRelayout(TextView.java:5378)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2688)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2556)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2531)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.BaseFragment.updateButtonText(BaseFragment.java:65)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.WrongFragment.onSharedPreferenceChanged(WrongFragment.java:12)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.app.ContextImpl$SharedPreferencesImpl$EditorImpl.commit(ContextImpl.java:2830)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.BaseFragment$1$1.run(BaseFragment.java:36)
09-05 07:16:58.993: E/AndroidRuntime(403): at java.lang.Thread.run(Thread.java:1096)

Fortunately the obvious crashlog is obvious and you can solve this issue in about 5 seconds, by wrapping the UI action in Activity.runOnUiThread() method. Morbidly curious may track the root cause of this issue in GrepCode. Tl;dr: before Gingerbread the listeners are notified in the same thread as the SharedPreferences.commit() caller, in later releases commit() ensures the notifications are performed in UI thread.

Code of the sample application that demonstrates this issue is available on my github.

Thursday, August 30, 2012

Custom scrollbar graphics in Android

One of the UI elements you might want to customize in your app are scrollbars for ListViews, ScrollViews and other scrollable objects. It's a minor visual change, but it might give your app more consistent and polished look and feel, especially in case of the heavily branded UI.

Anyways, changing the default scrollbar style is dead simple - you just need to specify new drawables for the track and thumb in your style:

<style name="badList">
    <item name="android:scrollbarThumbVertical">@drawable/scrollbar_handle</item>
    <item name="android:scrollbarTrackVertical">@drawable/scrollbar_track</item>
</style>


Of course, there is a small gotcha (why else would I bother to write this blog post?). Let's say that you don't need to customize horizontal scrollbar, so you prepare only vertical 9-patches:


On Ice Cream Sandwich everything looks fine, but on Gingerbread the graphics are not exactly what you want:



Quick Google search returned a StackOverflow thread with a description and link to Android bug tracker, but no full workaround. If you're too lazy to click on those links, on Gingerbread and earlier releases the View asks ScrollbarDrawable for the height of horizontal scrollbar and uses it as a horizontal scrollbar height and vertical scrollbar width. Let's modify our scrollbar graphics a bit:



And apply it as both horizontal and vertical scrollbar.

<style name="goodList">
    <item name="android:scrollbarThumbVertical">@drawable/scrollbar_handle</item>
    <item name="android:scrollbarTrackVertical">@drawable/scrollbar_track</item>
    <item name="android:scrollbarThumbHorizontal">@drawable/scrollbar_handle</item>
    <item name="android:scrollbarTrackHorizontal">@drawable/scrollbar_track</item>
</style>

Lo and behold, it works!





Note: in general case you probably want to create another graphics for horizontal scrollbar by rotating and flipping vertical scrollbar graphics. Our scrollbar graphics doesn't have any non-symmetric elements and I'm lazy, so I used the same 9-patch for both scrollbars.

The code of the sample application can be found here.

Friday, June 22, 2012

SQLite unions gotcha

Recently I've been tracking the problem with SQLite database used in the Android application I'm working on. The starting point of the whole story is that I've noticed that the cursor created with the SQLiteDatabase.query() method returned smaller data set than the same query executed through sqlite3 command line interface. The query in question looked like this:
SELECT * FROM some_view WHERE (column_a=1 OR column_b=1);
Inside the Android app I was getting rows for the second part of OR clause (i.e. column_b=1), but no rows for the first part.

Quick search through Android sources yielded the clue - I wasn't executing exactly the same query on the command line. Selection arguments are always bound as a strings, so the question marks in query string should be surrounded with quotes. So the Android app was executing the following query:
SELECT * FROM some_view WHERE (column_a="1" OR column_b="1");
So now we have another puzzle: why column_b=1 and column_b="1" give the same results, but the behavior is different for column_a?
Let's try to reproduce the problem:
sqlite> .mode column
sqlite> .headers on
sqlite> CREATE TABLE t (x INTEGER);
sqlite> INSERT INTO t VALUES(1);
sqlite> SELECT COUNT(*) FROM t WHERE x=1;
1
sqlite> SELECT COUNT(*) FROM t WHERE x="1";
1
So far so good, no surprises. Let's create a view similar to the one which causes problems.
sqlite> CREATE VIEW v AS SELECT NULL AS a, x AS b FROM t UNION SELECT x, NULL FROM t;
sqlite> SELECT * FROM v;
a           b
----------  ----------
            1
1
Now let's take a look at counts:
sqlite> SELECT COUNT(*) FROM v WHERE b=1;
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM v WHERE b="1";
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM v WHERE a=1;
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM v WHERE a="1";
COUNT(*)
----------
0
Yay, we reproduced our bug. But why is this happening?
sqlite> PRAGMA TABLE_INFO(v);
cid         name        type        notnull     dflt_value  pk
----------  ----------  ----------  ----------  ----------  ----------
0           a                       0                       0
1           b           integer     0                       0
It seems that the lack of explicitly defined type of the first column prevents type conversion (please note that this is only my assumption based on the observations above; unfortunately the sqlite documentation doesn't cover such cases in detail). How can we work around this issue?
sqlite> CREATE VIEW vfix AS SELECT x AS a, x AS b FROM t WHERE 1=0 UNION SELECT * FROM v;
sqlite> PRAGMA TABLE_INFO(vfix);
cid         name        type        notnull     dflt_value  pk
----------  ----------  ----------  ----------  ----------  ----------
0           a           integer     0                       0
1           b           integer     0                       0
As you can see the column types are correctly copied from the underlying table. Let's check the counts:
sqlite> SELECT COUNT(*) FROM vfix WHERE b=1;
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM vfix WHERE b="1";
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM vfix WHERE a=1;
COUNT(*)
----------
1
sqlite> SELECT COUNT(*) FROM vfix WHERE a="1";
COUNT(*)
----------
1
Looks OK. Pretty? No, but it does the job and that's what matters at the end of the day.

Wednesday, March 14, 2012

Large raw assets in Android 2.2

My most recent application, Word Judge, contains full dictionary of word valid in word games. I went to great lengths to minimize the size of this data, but for some languages it's still quite large. For example polish dictionary is compressed from 35MB to 1.4MB. In Android 2.2 and earlier, if you add such large file as an asset and then try to open it, you'll get IOException. The exception itself doesn't contain any useful information, but the following text appears in logcat:
03-07 14:40:42.345: D/asset(301): Data exceeds UNCOMPRESS_DATA_MAX (1442144 vs 1048576)
With that information googling the workaround is easy. Some folks recommend splitting the file into multiple parts, or repackaging the apk, but the simplest solution is to change the file extension of your asset to one of the extensions listed in aapt tool sources as files that are not compressed by default:
/* these formats are already compressed, or don't compress well */
static const char* kNoCompressExt[] = {
    ".jpg", ".jpeg", ".png", ".gif",
    ".wav", ".mp2", ".mp3", ".ogg", ".aac",
    ".mpg", ".mpeg", ".mid", ".midi", ".smf", ".jet",
    ".rtttl", ".imy", ".xmf", ".mp4", ".m4a",
    ".m4v", ".3gp", ".3gpp", ".3g2", ".3gpp2",
    ".amr", ".awb", ".wma", ".wmv"
};
Of course if you target API level 9 or newer, you don't have to worry about this gotcha.

There is one more thing worth mentioning: the speed of IO operations in debug mode is atrocious. Loading the 1.4MB dictionary to memory takes over 10 seconds. Fortunately when the program is running without debugger attached, the loading time decreases to less than 1 second.

Tuesday, February 28, 2012

WP7 WebRequest oddity

During testing of a Windows Phone 7 application I write during my day job I noticed strange thing: when I disabled network connection, I received 404 NotFound error. More precisely, the WebRequest threw WebException with Status = UnknownError, Response.Uri = {} and Response.StatusCode = NotFound.

So you get 404 in two cases: when either connection endpoint is down or when server actually responds with 404 NotFound. It would be nice to separate those two cases though, for example to display to the user the message that actually helps them fix the problem.


Fortunately you can tell those two situations apart by checking WebException.Response.ResponseUri - in case of connection failure it contains empty Uri object (not null, just empty). Here's the extension method I use to convert the exception to the one that makes more sense to me :
public static WebResponse SaneEndGetResponse(this WebRequest request, IAsyncResult asyncResult)
{
    try
    {
        return request.EndGetResponse(asyncResult);
    }
    catch (WebException wex)
    {
        if (wex.Response != null &&
            ((HttpWebResponse)wex.Response).StatusCode == HttpStatusCode.NotFound &&
            wex.Response.ResponseUri != null &&
            String.IsNullOrEmpty(wex.Response.ResponseUri.ToString()))
        {
            throw new WebException("Network error", WebExceptionStatus.ConnectFailure);
        }
        throw;
    }
}

Wednesday, February 22, 2012

Android LayoutInflater gotcha

Many custom Adapter tutorials contain subtle error which can be hard to find and fix. Even efficient list adapter sample from Android SDK contains this bug. If you compile and run the sample without any changes you should see something like this:


There's nothing wrong with the list on this screenshot, as long as that's the look you want. But what if you want the list items to be wider? Let's change the list item layout:

<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright (C) 2007 The Android Open Source Project

     Licensed under the Apache License, Version 2.0 (the "License");
     you may not use this file except in compliance with the License.
     You may obtain a copy of the License at
  
          http://www.apache.org/licenses/LICENSE-2.0
  
     Unless required by applicable law or agreed to in writing, software
     distributed under the License is distributed on an "AS IS" BASIS,
     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
     See the License for the specific language governing permissions and
     limitations under the License.
-->

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:orientation="horizontal"
    android:layout_width="match_parent"
    android:layout_height="150dip">

    <ImageView android:id="@+id/icon"
        android:layout_width="48dip"
        android:layout_height="48dip" />

    <TextView android:id="@+id/text"
        android:layout_gravity="center_vertical"
        android:layout_width="0dip"
        android:layout_weight="1.0"
        android:layout_height="wrap_content" />

</LinearLayout>

What's changed? Nothing, nada, zilch, zip. No changes whatsoever:


You can use hierarchy viewer tool to verify that the list item height was not set correctly.


There's obviously nothing wrong with the layout xml, so let's take a look at the code:

/*
 * Copyright (C) 2008 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.example.android.apis.view;

import android.app.ListActivity;
import android.content.Context;
import android.os.Bundle;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.BaseAdapter;
import android.widget.TextView;
import android.widget.ImageView;
import android.graphics.BitmapFactory;
import android.graphics.Bitmap;
import com.example.android.apis.R;

/**
 * Demonstrates how to write an efficient list adapter. The adapter used in this example binds
 * to an ImageView and to a TextView for each row in the list.
 *
 * To work efficiently the adapter implemented here uses two techniques:
 * - It reuses the convertView passed to getView() to avoid inflating View when it is not necessary
 * - It uses the ViewHolder pattern to avoid calling findViewById() when it is not necessary
 *
 * The ViewHolder pattern consists in storing a data structure in the tag of the view returned by
 * getView(). This data structures contains references to the views we want to bind data to, thus
 * avoiding calls to findViewById() every time getView() is invoked.
 */
public class List14 extends ListActivity {

    private static class EfficientAdapter extends BaseAdapter {
        private LayoutInflater mInflater;
        private Bitmap mIcon1;
        private Bitmap mIcon2;

        public EfficientAdapter(Context context) {
            // Cache the LayoutInflate to avoid asking for a new one each time.
            mInflater = LayoutInflater.from(context);

            // Icons bound to the rows.
            mIcon1 = BitmapFactory.decodeResource(context.getResources(), R.drawable.icon48x48_1);
            mIcon2 = BitmapFactory.decodeResource(context.getResources(), R.drawable.icon48x48_2);
        }

        /**
         * The number of items in the list is determined by the number of speeches
         * in our array.
         *
         * @see android.widget.ListAdapter#getCount()
         */
        public int getCount() {
            return DATA.length;
        }

        /**
         * Since the data comes from an array, just returning the index is
         * sufficent to get at the data. If we were using a more complex data
         * structure, we would return whatever object represents one row in the
         * list.
         *
         * @see android.widget.ListAdapter#getItem(int)
         */
        public Object getItem(int position) {
            return position;
        }

        /**
         * Use the array index as a unique id.
         *
         * @see android.widget.ListAdapter#getItemId(int)
         */
        public long getItemId(int position) {
            return position;
        }

        /**
         * Make a view to hold each row.
         *
         * @see android.widget.ListAdapter#getView(int, android.view.View,
         *      android.view.ViewGroup)
         */
        public View getView(int position, View convertView, ViewGroup parent) {
            // A ViewHolder keeps references to children views to avoid unneccessary calls
            // to findViewById() on each row.
            ViewHolder holder;

            // When convertView is not null, we can reuse it directly, there is no need
            // to reinflate it. We only inflate a new View when the convertView supplied
            // by ListView is null.
            if (convertView == null) {
                convertView = mInflater.inflate(R.layout.list_item_icon_text, null);

                // Creates a ViewHolder and store references to the two children views
                // we want to bind data to.
                holder = new ViewHolder();
                holder.text = (TextView) convertView.findViewById(R.id.text);
                holder.icon = (ImageView) convertView.findViewById(R.id.icon);

                convertView.setTag(holder);
            } else {
                // Get the ViewHolder back to get fast access to the TextView
                // and the ImageView.
                holder = (ViewHolder) convertView.getTag();
            }

            // Bind the data efficiently with the holder.
            holder.text.setText(DATA[position]);
            holder.icon.setImageBitmap((position & 1) == 1 ? mIcon1 : mIcon2);

            return convertView;
        }

        static class ViewHolder {
            TextView text;
            ImageView icon;
        }
    }

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setListAdapter(new EfficientAdapter(this));
    }

    private static final String[] DATA = Cheeses.sCheeseStrings;
}

I've highlighted the problematic line. It turns out that you have to use another overload of LayoutInflater.inflate method:

convertView = mInflater.inflate(R.layout.list_item_icon_text, parent, false);

We set the attachToRoot to false, because we just want to properly intialize LayoutParams for the LinearLayout of the item and let the ListView to add the inflated views wherever it needs. In fact, setting it to true causes exception to be thrown from AdapterView:

java.lang.UnsupportedOperationException: addView(View, LayoutParams) is not supported in AdapterView
    at android.widget.AdapterView.addView(AdapterView.java:461)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:416)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:320)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:276)
    at com.fu.InflaterBugActivity$EfficientAdapter.getView(InflaterBugActivity.java:77)
    at android.widget.AbsListView.obtainView(AbsListView.java:1430)
    at android.widget.ListView.makeAndAddView(ListView.java:1745)
    at android.widget.ListView.fillDown(ListView.java:670)
    at android.widget.ListView.fillFromTop(ListView.java:727)
    at android.widget.ListView.layoutChildren(ListView.java:1598)
    at android.widget.AbsListView.onLayout(AbsListView.java:1260)
    at android.view.View.layout(View.java:7175)
    at android.widget.FrameLayout.onLayout(FrameLayout.java:338)
    at android.view.View.layout(View.java:7175)
    at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1254)
    at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1130)
    at android.widget.LinearLayout.onLayout(LinearLayout.java:1047)
    at android.view.View.layout(View.java:7175)
    at android.widget.FrameLayout.onLayout(FrameLayout.java:338)
    at android.view.View.layout(View.java:7175)
    at android.view.ViewRoot.performTraversals(ViewRoot.java:1140)
    at android.view.ViewRoot.handleMessage(ViewRoot.java:1859)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:123)
    at android.app.ActivityThread.main(ActivityThread.java:3683)
    at java.lang.reflect.Method.invokeNative(Native Method)
    at java.lang.reflect.Method.invoke(Method.java:507)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597)
    at dalvik.system.NativeStart.main(Native Method)
Here's the application screenshot after the change:

Ugly, but that's exactly what we should get after setting layout height to 150dip.