Problem: Chapter 7 "Saving Your To-Do List" (Page 218)

DrGaribaldi · June 16th, 2010, 01:15 AM

This example seems broken to me. It makes an unnatural association between the primary key of database items and the location of the associated ToDoItems in the ListView.

Suppose that I start with a new, empty database. I add a new ToDoItem, which creates an entry in the database and automatically assigns its _id=1. I add a second ToDoItem, which creates another entry in the database and automatically assigns its _id=2. If I delete the first item (with _id=1), it leaves only one item in the database (_id=2). However, at this point, the index of the only item in the ListView is 1 not 2. If I try to remove that item, it will try to delete an entry in the database with _id=1 and no such item exists. What's more, if I add a new ToDoItem, when it's added to the database, it will automatically get assigned _id=3, but again its position in the ListView will not be 3.

To fix this, you'll probably want to add a field to the ToDoItem class that represent the _id value from the database. When you select an item to delete, extract the id from the selected item in the ListView, then use that id value as the row index when you call removeTask() on the database adapter.

The author's code provided on the website also exhibits the same incorrect behavior.

schmide · March 21st, 2011, 01:09 AM

*** See next post ***

I had to make it work. It may not be the cleanest code.

Code:

  // Remove a task based on its index
  public boolean removeTask(long _rowIndex) {
	Cursor tempCursor = db.query(DATABASE_TABLE, 
            new String[] { KEY_ID }, 
            null, null, null, null, null);
	if(tempCursor.moveToPosition((int)_rowIndex-1)) 
		return db.delete(DATABASE_TABLE, KEY_ID + "=" + tempCursor.getString(0), null) > 0;
	else
		return false;	
  }

schmide · March 21st, 2011, 01:53 AM

I thought about it and this would be a better solution. It basically adds a function to getRowStringFromIndex that returns the appropriate string to complete the query. Appropriate following/effected functions fixed and included.

Code:

  public String getRowStringFromIndex(long _rowIndex) {
		Cursor tempCursor = db.query(DATABASE_TABLE, 
	            new String[] { KEY_ID }, 
	            null, null, null, null, null);
		if(tempCursor.moveToPosition((int)_rowIndex-1)) 
			return tempCursor.getString(0);
		else 
			return "";
  }
  
  // Remove a task based on its index
  public boolean removeTask(long _rowIndex) {
		return db.delete(DATABASE_TABLE, KEY_ID + "=" + getRowStringFromIndex(_rowIndex), null) > 0;
  }

  // Update a task
  public boolean updateTask(long _rowIndex, String _task) {
    ContentValues newValue = new ContentValues();
    newValue.put(KEY_TASK, _task);
    return db.update(DATABASE_TABLE, newValue, KEY_ID + "=" + getRowStringFromIndex(_rowIndex), null) > 0;
  }
  
  public Cursor getAllToDoItemsCursor() {
    return db.query(DATABASE_TABLE, 
                    new String[] { KEY_ID, KEY_TASK, KEY_CREATION_DATE}, 
                    null, null, null, null, null);
  }

  public Cursor setCursorToToDoItem(long _rowIndex) throws SQLException {
	String rowIndexString = getRowStringFromIndex(_rowIndex);
    Cursor result = db.query(true, DATABASE_TABLE, 
	                           new String[] {KEY_ID, KEY_TASK},
                             KEY_ID + "=" + rowIndexString, null, null, null, 
                             null, null);
    if ((result.getCount() == 0) || !result.moveToFirst()) {
      throw new SQLException("No to do items found for row: " + rowIndexString);
    }
    return result;
  }

  public ToDoItem getToDoItem(long _rowIndex) throws SQLException {
	String rowIndexString = getRowStringFromIndex(_rowIndex);
    Cursor cursor = db.query(true, DATABASE_TABLE, 
                             new String[] {KEY_ID, KEY_TASK},
                             KEY_ID + "=" + rowIndexString, null, null, null, 
                             null, null);
    if ((cursor.getCount() == 0) || !cursor.moveToFirst()) {
      throw new SQLException("No to do item found for row: " + rowIndexString);
    }

    String task = cursor.getString(cursor.getColumnIndex(KEY_TASK));
    long created = cursor.getLong(cursor.getColumnIndex(KEY_CREATION_DATE));
		  
    ToDoItem result = new ToDoItem(task, new Date(created));
    return result;  
  }