yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

Suggestion - add a fetchByQuery to get the list of items #229

Closed MFlisar closed 8 years ago

MFlisar commented 8 years ago

Currently I'm having following helper functions:

 public static <T extends TableModel> ArrayList<T> getAll(SqlTable<TableModel> table, Class<T> clazz, Order... order)
{
    ArrayList<T> data = new ArrayList<>();

    try
    {
        Query query = Query.select().from(table);
        if (order != null)
            query.orderBy(order);
        SquidCursor<T> cursor = MainApp.getDB().query(clazz, query);
        cursor.moveToFirst();
        while (!cursor.isAfterLast())
        {
            Constructor<?> ctor = clazz.getConstructor();
            T o = (T) ctor.newInstance(new Object[]{});
            o.readPropertiesFromCursor(cursor);
            data.add(o);
            cursor.moveToNext();
        }
        cursor.close();
    }
    catch (NoSuchMethodException e)
    {
        L.e(DBFunctions.class, e);
    }
    catch (IllegalAccessException e)
    {
        L.e(DBFunctions.class, e);
    }
    catch (InvocationTargetException e)
    {
        L.e(DBFunctions.class, e);
    }
    catch (InstantiationException e)
    {
        L.e(DBFunctions.class, e);
    }
    return data;
}

I think a simple List<Data> data = database.fetchAllByQuery(query); would be nice additional function to the fetchByQuery which returns the first match...

jdkoren commented 8 years ago

Thus far we've tried to be more of a wrapper around Android's SQLiteOpenHelper and SQLiteDatabase, which return Cursors. We also want to leave the implementation open to developers; some may want to just convert an entire Cursor to a List up front (as you have suggested), whereas others might want to inflate the models lazily, perhaps as part of a Loader or some other mechanism.

Having a simple utility method in SquidUtilities to convert a SquidCursor into a List might be a nice addition though. For the sake of keeping SquidDatabase's API clean, I'm more inclined to do that over adding too many overloads/variations of query.

Until then, for your use case, it's easy enough to add the method to your SquidDatabase subclass, or in a utility class if you have multiple SquidDatabase subclasses and don't want to duplicate that code. If you want to avoid the reflection, you could instead pass in an empty instance of the model instead of passing the class itself and use clone() to produce new instances.

MFlisar commented 8 years ago

Ok, understand. Still I think this common functions would fit into an library internal utility class as well.

Thanks for the tip with the empty instance to avoid reflection. Now I understand why you use the emtpy instance as paramater in other functions as well, did not understand that until now, as I have never used it that way.