AsyncTask, Android, and SQL

I originally posted this in stackoverflow.com but the question may be too broad. I'm trying to figure out the best way to download and use an SQL database from my server. I have included the code i whipped up but I'm not sure if it's a viable way to accomplish this so peer review would be extremely helpful :)

As it stands now, the database will be downloaded in a separate thread but when UI components are initialized they fail (obviously, as the database doesnt exist while its still being downloaded).

```
package com.sandbox.databaseone;

import java.io.BufferedReader;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.io.InputStreamReader;

import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;

import android.database.sqlite.SQLiteDatabase;
import android.os.AsyncTask;
import android.util.Log;

public class DatabaseManager {

private SQLiteDatabase database;
private int currentVersion = 0;
private int nextVersion = 0;

private static String databasePath = "/data/data/com.sandbox.databaseone/databases/";
private static String databaseFile = "dbone.sqlite";

private static String databaseBaseURL = "http://www.redstalker.com/dbone/";
private static String databaseVersionURL = "version.txt";

public DatabaseManager()
{
database = null;
}

public void initialize()
{
DatabaseVersionCheck check = new DatabaseVersionCheck();

String url = databaseBaseURL + databaseVersionURL;

check.execute(url);
}

private void init_database(String path)
{
database = SQLiteDatabase.openDatabase(path, null, SQLiteDatabase.OPEN_READONLY);

if(database != null)
{
currentVersion = nextVersion;
}
else
{
nextVersion = 0;
}

}

private class DatabaseVersionCheck extends AsyncTask
{

@Override
protected String doInBackground(String... params) {

Some generic Java notes, since I'm not too familiar with Android.

-
databasePath, databaseFile, databaseBaseURL, databaseVersionURL should be constants (all uppercase with words separated by underscores):

private static final String DATABASE_PATH = 
    "/data/data/com.sandbox.databaseone/databases/";
private static final String DATABASE_FILE = "dbone.sqlite";
private static final String DATABASE_BASE_URL = 
    "http://www.redstalker.com/dbone/";
private static final String DATABASE_VERSION_URL =
    "version.txt";

Reference: Code Conventions for the Java Programming Language, 9 - Naming Conventions

-
According to the previous Code Conventions, init_database should be initDatabase.

-
If databaseBaseURL and databaseVersionURL hadn't be constant I'd named them as databaseBaseUrl and databaseBaseVersionUrl. From Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions:


While uppercase may be more common,
a strong argument can made in favor of capitalizing only the first
letter: even if multiple acronyms occur back-to-back, you can still
tell where one word starts and the next word ends.
Which class name would you rather see, HTTPURL or HttpUrl?

-

public DatabaseManager()
{
    database = null;
}

Initializing fields with null is unnecessary since null is the default value of references.

-
I'd call the StringBuilder in the doInBackground method as result.

StringBuilder builder = new StringBuilder();

It would say what's the purpose of the object. For the same reason I'd rename the

• boolean result to boolean success,

• in to responseStream, and

• d to downloadDatabase.

-
Close your streams in a finally block. In case of a previous errors they won't be closed.

-
The code does databasePath + databaseFile more than once. Create a method for that.

-
I don't know if it is applicable to Android or not, but in Java it's a good practice to pass the character set to the constructor of InputStreamReader. Without this InputStreamReader uses the default charset which could vary from system to system.

BufferedReader reader = 
    new BufferedReader(new InputStreamReader(in, "UTF-8"));

-
I'd use Commons IO's IOUtils for the copying. It has a copy(InputStream input, OutputStream output) which you could use in the DownloadDatabase task and you could use copy(InputStream input, Writer output) in the DatabaseVersionCheck task if you replace the StringBuilder to a StringWriter.

-
In the DatabaseVersionCheck.onPostExecute I'd use guard clauses:

@Override
protected void onPostExecute(String result) {
    if(result == null) {
        return;
    }

    int version = Integer.parseInt(result);
    if(version <= currentVersion) {
        return;
    }

    nextVersion = version;

    DownloadDatabase d = new DownloadDatabase();
    d.execute();
}

It makes the code flatten and more readable. References: Replace Nested Conditional with Guard Clauses in Refactoring: Improving the Design of Existing Code; Flattening Arrow Code

-
e.printStackTrace() is not the best practice. Maybe you should inform the user about the error and/or log it.

• Why is exception.printStackTrace() considered bad practice?

• Is it a bad idea to use printStackTrace() in Android Exceptions?

-
Integer.parseInt could throw NumberFormatException. Maybe the code should handle it.

StackExchange Code Review Q#10715, answer score: 4