Pulling from data from SQLite DB on Android

Question

Apart from merging the 2 get methods, what else can I do to optimise this? Both input and output cannot be changed. Is there a way to iterate a map while writing to it?

// String condition - "cust" for customer group, "item" for itemGroup
public HashMap<String, ArrayList<PriceTable>> getPromoForViewGroup(
            String condition) {
    String custGroup = "custType";
    String itemGroup = "itemType";
    HashMap<String, ArrayList<PriceTable>> valueMap = new HashMap<String, ArrayList<PriceTable>>();

    if (condition.equals(CUST)) {
        getPromoForViewGroupHelperCust(custGroup, condition, valueMap);
    }

    if (condition.equals(ITEM)) {
        getPromoForViewGroupHelperItem(itemGroup, condition, valueMap);

    }

    return valueMap;
}

private Map<String, ArrayList<PriceTable>> getPromoForViewGroupHelperItem(
            String searchGroup, String condition,
            Map<String, ArrayList<PriceTable>> valueMap) {

    List<PriceTable> allList = new ArrayList<PriceTable>();

    Cursor cursor = database.rawQuery("Select * from PriceTable where "
            + searchGroup + " IS NOT NULL", null);
    cursor.moveToFirst();
    while (!cursor.isAfterLast()) {
        PriceTable pt = cursorToPriceTable(cursor);
        if (!pt.getItemType().equals(""))
            allList.add(pt);

        cursor.moveToNext();

    }

    for (PriceTable pt1 : allList) {
        ArrayList<PriceTable> set = valueMap.get(pt1.getItemType());
        if (set == null) {
            set = new ArrayList<PriceTable>();
            set.add(pt1);
        } else {
            set.add(pt1);
        }
        valueMap.put(pt1.getItemType(), set);

    }
    cursor.close();

    return valueMap;

}

private Map<String, ArrayList<PriceTable>> getPromoForViewGroupHelperCust(
        String searchGroup, String condition,
        Map<String, ArrayList<PriceTable>> valueMap) {

    List<PriceTable> allList = new ArrayList<PriceTable>();

    Cursor cursor = database.rawQuery("Select * from PriceTable where "
            + searchGroup + " IS NOT NULL", null);
    cursor.moveToFirst();
    while (!cursor.isAfterLast()) {
        PriceTable pt = cursorToPriceTable(cursor);
        if (!pt.getCustType().equals(""))
            allList.add(pt);

        cursor.moveToNext();

    }

    for (PriceTable pt1 : allList) {
        ArrayList<PriceTable> set = valueMap.get(pt1.getCustType());
        if (set == null) {
            set = new ArrayList<PriceTable>();
            set.add(pt1);
        } else {
            set.add(pt1);
        }
        valueMap.put(pt1.getCustType(), set);

    }
    cursor.close();
    return valueMap;

}


private PriceTable cursorToPriceTable(Cursor cursor) {
        PriceTable priceTable = new PriceTable();

        priceTable.setPromoNum(cursor.getString(0));
        priceTable.setPromoType(cursor.getString(1));
        priceTable.setPromoRank(cursor.getString(2));
        priceTable.setPromoName(cursor.getString(3));
        priceTable.setItemType(cursor.getString(4));
        priceTable.setItemCode(cursor.getString(5));
        priceTable.setCustType(cursor.getString(6));
        priceTable.setCustCode(cursor.getString(7));
        priceTable.setStartDate(cursor.getString(8));
        priceTable.setEndDate(cursor.getString(9));
        priceTable.setPercent(cursor.getString(10));
        priceTable.setSalesPrice(cursor.getString(11));

        return priceTable;
    }

Answer 1

Updating collections

Many things can be simplified around this snippet:

for (PriceTable pt1 : allList) {
    ArrayList<PriceTable> set = valueMap.get(pt1.getItemType());
    if (set == null) {
        set = new ArrayList<PriceTable>();
        set.add(pt1);
    } else {
        set.add(pt1);
    }
    valueMap.put(pt1.getItemType(), set);

}
cursor.close();

This is equivalent:

for (PriceTable pt1 : allList) {
    String itemType = pt1.getItemType();
    ArrayList<PriceTable> set = valueMap.get(itemType);
    if (set == null) {
        set = new ArrayList<PriceTable>();
        valueMap.put(itemType, set);
    }
    set.add(pt1);
}

What I did:

• Moved set.add(pt1); out of the if-else
• Moved valueMap.put(itemType, set); inside the if
• No need for an else
• Extracted pt1.getItemType() to avoid an extra lookup
• cursor.close(); doesn't belong here, it should be done before this snippet. It makes the code easier to read if you don't delay actions like this.

Use interfaces types instead of implementations

When the interface type is enough, you should use that instead of the specific implementation type. For example instead of this:

public HashMap<String, ArrayList<PriceTable>> getPromoForViewGroup(
            String condition) {
    String custGroup = "custType";
    String itemGroup = "itemType";
    HashMap<String, ArrayList<PriceTable>> valueMap = new HashMap<String, ArrayList<PriceTable>>();

This is recommended:

public Map<String, List<PriceTable>> getPromoForViewGroup(String condition) {
    String custGroup = "custType";
    String itemGroup = "itemType";
    Map<String, List<PriceTable>> valueMap = new HashMap<String, List<PriceTable>>();

Notice all the rewritten types.

Mutually exclusive if-else

This conditions will not be both true at the same time, so instead of two separate if, they should be joined with an else if:

if (condition.equals(CUST)) {
    getPromoForViewGroupHelperCust(custGroup, condition, valueMap);
}

if (condition.equals(ITEM)) {
    getPromoForViewGroupHelperItem(itemGroup, condition, valueMap);

}

Don't Select *

It's not recommended to select columns using * wildcard. Different versions of the database might have columns in different order than you expect. There might be more columns than you expected, selecting more data than necessary, causing slowness. It's recommended to name the columns you need explicitly, and access their values by name.

Misc

Apart from merging the 2 get methods, what else can I do to optimise this?

Oh definitely merge those methods...

Is there a way to iterate a map while writing to it?

Kind of. You can iterate over a copy, while writing to the original. For example:

Map<String, Integer> map = new HashMap<>();
map.put("hello", 2);
map.put("world", 7);
for (Map.Entry<String, Integer> entry : new HashMap<>(map).entrySet()) {
    if (entry.getValue() > 5) {
        map.remove(entry.getKey());
    }
}
System.out.println(map);

Prints {hello=2}.