Calculating company-wise total expenses

Question

I have a variable List. Its declaration is like :

List<HashMap<String, Object>>

Sample data in it:

[{total-expense=228, company=HiHard, year=2009}, {total-expense=2936, company=POTENT, year=2009}, {total-expense=1362412.65, company=HiHard, year=2010}, {total-expense=9007.96, company=POTENT, year=2010}, {total-expense=240427.43, company=HiHard, year=2011}, {total-expense=1956.11, company=POTENT, year=2011}]

I have written a program to fetch the below result:

{"HiHard":[[228,1362412.65,240427.43]],"POTENT":[[2936,9007.96,1956.11]]}

i.e. to fetch company-wise total expense incurred for financial years 2009, 2010, 2011.

Current program logic:

1) getDistinctCompanies from the list

2) Iterate through the distinct companies[from step 1] and get the expense incurred by passing in company and list.

Complete program:

public class JSONConstruction {
    public static void main(String[] args) {
        List<HashMap<String, Object>> list = new ArrayList<HashMap<String, Object>>();

        ... ASSUME list has the SAMPLE data as described before.

        List<String> companyList = getDistinctCompanies(list);

        Iterator<String> iterCompany = companyList.iterator();
        String company;
        JSONObject obj = new JSONObject();
        while(iterCompany.hasNext()){
            company = iterCompany.next();
            JSONArray jsonList = new JSONArray();
            jsonList.add(getExpenseList(list, company));
            obj.put(company, jsonList);
        }

        System.out.print(obj);
    }

    private static List<String> getDistinctCompanies(List<HashMap<String, Object>> list) {
        Iterator<HashMap<String, Object>> iterList = list.iterator();
        String company;
        List<String> companyList = new ArrayList<String>();
        while (iterList.hasNext()) {
            HashMap<String, Object> map = iterList.next();
            company = (String) map.get("company");
            if(companyList==null || (!companyList.contains(company))) {
                companyList.add(company);
            }
        }
        return companyList;
    }

    private static List<Object> getExpenseList(List<HashMap<String, Object>> list, String company) {
        Iterator<HashMap<String, Object>> iterList = list.iterator();
        List<Object> expenseList = new ArrayList<Object>();
        String companyMatch;
        while (iterList.hasNext()) {
            HashMap<String, Object> map = iterList.next();
            companyMatch = (String) map.get("company");
            if(companyMatch.equals(company)) {
                Object obj = map.get("total-expense");
                expenseList.add(obj);
            }
        }
        return expenseList;
    }

Though the data is fetched correctly, I am eager to know a better way of coding this program.

Answer 1

Interfaces over implementations type declaration

List<HashMap<String, Object>> list = new ArrayList<HashMap<String, Object>>();

Favoring interfaces over implementations for type declaration applies to nested generic types as well, meaning the above can be better represented as:

List<Map<String, Object>> list = new ArrayList<Map<String, Object>>();
// or Java 7 and greater
List<Map<String, Object>> list = new ArrayList<>();

Enforcing distinct values

A Set instead of a List makes for a better representation of a distinct collection of values, as its API mandates no duplicate elements. As such, your getDistinctCompanies() method should return a Set. In addition, the if-check is redundant as you can freely call the add() method with the guarantee that duplicate elements will not be added:

private static Set<String> getDistinctCompanies(List<HashMap<String, Object>> list) {
    Set<String> companyList = new HashSet<>();
    // Using for-each loop
    for (Map<String, Object> map : list) {
        companyList.add((String) map.get("company"));
    }
    return companyList;
}

Java 8

Processing a Collection of elements can often be made simpler using Java 8's streams.

For your use case, what you are doing are just:

1. From each Map element of list, extract the company and total-expense values.
2. Group total-expense into a List by the company names.
3. Use each company name as the key of a JSONObject, with a JSONArray wrapper over the total-expense values as the value.

You can generate an intermediary Map<String, List<Object> result to represent the one-to-many company \$\to\$ total-expense processing from steps 1 and 2:

private static Map<String, List<Object>> getTotalExpensesByCompany(
        List<Map<String, Object>> list) {
    return list.stream()
            .map(m -> Collections.singletonMap((String) m.get("company"), 
                                                m.get("total-expense")))
            .map(Map::entrySet)
            .flatMap(Set::stream)
            .collect(Collectors.groupingBy(Entry::getKey,
                        Collectors.mapping(Entry::getValue, Collectors.toList())));
}

The map() 'converts' each stream element as the desired company \$\to\$ total-expense mapping, before flatMap()-ping to a Stream of Entry objects. This is then collected into the resulting Map groupingBy() the Entry's key (using Entry::getKey as a method reference), and mapping() all the values (Entry::getValue) toList().

JSONObject result = new JSONObject();
Map<String, List<Object>> map = getTotalExpensesByCompany(list);
map.forEach((k ,v) -> result.put(k, new JSONArray().put(v)));

Step 3 is to call your JSONObject/JSONArray operations forEach() of the key-value entries in the Map, as shown above. A more succinct way can also be:

JSONObject result = new JSONObject();
getTotalExpensesByCompany(list).forEach((k ,v) -> result.put(k, new JSONArray().put(v)));

Also, if @200_success's comment is right and what you need is actually a one-dimensional list for JSONArray, use new JSONArray(v) instead.

Answer 2

private static List<String> getDistinctCompanies(List<HashMap<String, Object>> list) {
    Iterator<HashMap<String, Object>> iterList = list.iterator();
    String company;
    List<String> companyList = new ArrayList<String>();
    while (iterList.hasNext()) {
        HashMap<String, Object> map = iterList.next();
        company = (String) map.get("company");
        if(companyList==null || (!companyList.contains(company))) {
            companyList.add(company);
        }
    }
    return companyList;
}  

• There is no need to check if companyList==null because that won't ever happen because 4 lines prior you create the list.

• The scope of map is defined to be inside the while loop, but company is defined outside whereas it is only used in the while loop as well.

• You should name your variables proper. Shortening names doesn't add any value. Instead of iterList a simple iterator or listIterator would be much better.

• You should leave your variables and operators some space to breathe. This helps to make your code more readable. companyList==null vs companyList == null.