Why is this variable set to empty string when it is already initialized to an empty string?

311 views Asked by At

I have taken the following code snippet from the 5th snippet on this developer guide on Content Providers.

The confusion is that in the first statement String[] mSelectionArgs = {""};, mSelectionArgs[0] IS set to "".

Then later if the mSearchString is empty (TextUtils.isEmpty(mSearchString)), then again mSelectionArgs[0] is assigned "".

So the question is that why are they setting it to an empty string when it is already initialized to an empty string?

/*
 * This defines a one-element String array to contain the selection argument.
 */
String[] mSelectionArgs = {""};

// Gets a word from the UI
mSearchString = mSearchWord.getText().toString();

// Remember to insert code here to check for invalid or malicious input.

// If the word is the empty string, gets everything
if (TextUtils.isEmpty(mSearchString)) {
    // Setting the selection clause to null will return all words
    mSelectionClause = null;
    mSelectionArgs[0] = "";

} else {
    // Constructs a selection clause that matches the word that the user entered.
    mSelectionClause = UserDictionary.Words.WORD + " = ?";

    // Moves the user's input string to the selection arguments.
    mSelectionArgs[0] = mSearchString;

}
...
2

There are 2 answers

5
Vasiliy On BEST ANSWER

Except for additional clarity and code readability, as noted in another answer, this coding style makes for a less error prone code which is easier to maintain.

This way, if the initial value of mSelectionArgs is changed, or new code added which overrides this value before the execution of the if-else block, the code of this block will still execute correctly. Without this "rudimentary" assignment, a change as described above could lead to a bug which would be very difficult to trace.

As a side note:

This specific code snippet is not that good (yes, I know it is from Android Developers site...) - if you pass null as selection argument to query(), then it is better to also pass null as selectionArgs argument. I'd modify this sample to something like this (setting both selection and selectionArgs to null):

// Gets a word from the UI

mSearchString = mSearchWord.getText().toString();

// Remember to insert code here to check for invalid or malicious input.

String[] mSelectionArgs = null;

// If the word is the empty string, gets everything
if (TextUtils.isEmpty(mSearchString)) {
    // Setting the selection clause to null will return all words
    mSelectionClause = null;
    mSelectionArgs = null;

} else {
    // Constructs a selection clause that matches the word that the user entered.
    mSelectionClause = UserDictionary.Words.WORD + " = ?";

    // Moves the user's input string to the selection arguments.
    mSelectionArgs = new String[] {mSearchString};

}

Edit: why the above code snippet is better than the original one?

It is not an error to pass null as selection and non-null as selectionArgs. This array will be passed to the specific ContentProvider you're addressing, and shouldn't be used at all since selection does not contain any ? placeholders. Any ContentProvider violating this assumption is buggy. Although not an error, it just looks weird - why do you pass an object that should be ignored anyway? This also has performance cost (which is higher if ContentProvider runs in different process), which is proportional to the size of the object being passed.

Edit 2: why the above code snippet is MUCH better than the original one? Turns out that what I said above might be misleading. I found it out the hard way:

 Caused by: java.lang.IllegalArgumentException: Cannot bind argument at index 3 because the index is out of range.  The statement has 1 parameters.
        at android.database.sqlite.SQLiteProgram.bind(SQLiteProgram.java:212)
        at android.database.sqlite.SQLiteProgram.bindString(SQLiteProgram.java:166)
        at android.database.sqlite.SQLiteProgram.bindAllArgsAsStrings(SQLiteProgram.java:200)
        at android.database.sqlite.SQLiteDirectCursorDriver.query(SQLiteDirectCursorDriver.java:47)
        at android.database.sqlite.SQLiteDatabase.rawQueryWithFactory(SQLiteDatabase.java:1314)
        at android.database.sqlite.SQLiteDatabase.queryWithFactory(SQLiteDatabase.java:1161)
        at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1032)
        at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1200)

The above exception was thrown because I tried to pass selectionArgs which contained more elements than the number of ? placeholders in selection.

These two methods from SQLiteProgram.java are to "blame" for this exception:

public void bindAllArgsAsStrings(String[] bindArgs) {
    if (bindArgs != null) {
        for (int i = bindArgs.length; i != 0; i--) {
            bindString(i, bindArgs[i - 1]);
        }
    }
}

private void bind(int index, Object value) {
    if (index < 1 || index > mNumParameters) {
        throw new IllegalArgumentException("Cannot bind argument at index "
                + index + " because the index is out of range.  "
                + "The statement has " + mNumParameters + " parameters.");
    }
    mBindArgs[index - 1] = value;
}

Now, when I found out about this behavior, I think that the code example from Android Developers site is not just inefficient, but is a total crap!

Bottom line: if you pass null as selection, pass null as selectionArgs as well. If selection is not null and contains ? placeholders - make sure that the length of selectionArgs array equals the number of ? placeholders in selection.

0
ZhongYu On

I like it, because it's symmetric

if something
    var = x
else
    var = y

It's clear what var is under each condition, without needing to go back and visit its initial value.