-3

I am using a Map of type Map<Integer, HashMap<String, List<Employee>>>. After populating the map, if I immediately retrieve some values using the String key, the returned List<Employee> is correct. But after some iteration the values are changing.

After populating the map the first time, I have not used map.put anywhere, only used map.get().

private Map<Integer, HashMap<String, List<Employee>>> createDataMap(List<Employee> employeeList) {

    for(Employee e:employeeList){       
        Map<Integer, HashMap<String, List<Employee>>> dataMap = new HashMap<>();
        int level = Integer.parseInt(e.getLevel());
        HashMap<String, List<Employee>> detailsMap = dataMap.get(level);
        if (Objects.isNull(detailsMap)) {
            detailsMap = new HashMap<>();
            dataMap.put(level, detailsMap);
        }
        String deptName = key.getDepartment();
        List<Employee> keyList = detailsMap.get(deptName);
        if (Objects.isNull(keyList)) {
            keyList = new ArrayList<>();
            detailsMap.put(dataMapKey, keyList);
        }
        keyList.add(key);
    }
    return dataMap;
}
Bentaye
  • 9,403
  • 5
  • 32
  • 45
Ranji
  • 61
  • 1
  • 1
  • 6
  • 1
    Provide your code? – ChrisMcQueen Nov 28 '19 at 09:57
  • To simplify your code, you get use the ```getOrDefault``` of the class ```Map``` instead of testing if the value of your map is null after getting it – D. Lawrence Nov 28 '19 at 10:11
  • 5
    Your code doesn't make sense. `dataMap` is constructed per-employee *in the loop* then returned *outside* the loop. That won't even compile. In general, please look into `detailsMap = map.computeIfAbsent(level, HashMap::New)` which handles the conditional-creation logic – drekbour Nov 28 '19 at 10:44

1 Answers1

0
  • dataMap needs to be instantiated only once, outside of the loop.
  • When you use key it should be e, you should make sure whatever code you post compiles (as a minimum!)
  • detailsMap.put(dataMapKey, keyList); is not correct, should be detailsMap.put(e.getDepartment(), keyList);

I think that is what your code should look like. It now compiles and correctly creates your Map. I took the liberty to rename some variables to make it more readable.

private static Map<Integer, HashMap<String, List<Employee>>> createDataMap2(List<Employee> employeeList) {
    Map<Integer, HashMap<String, List<Employee>>> dataMap = new HashMap<>();

    for(Employee e:employeeList){

        // Get the departments for the employee's level
        int level = Integer.parseInt(e.getLevel());
        HashMap<String, List<Employee>> departmentsEmployeesMapForEmployeeLevel = dataMap.get(level);
        if (Objects.isNull(departmentsEmployeesMapForEmployeeLevel)) {
            departmentsEmployeesMapForEmployeeLevel = new HashMap<>();
            dataMap.put(level, departmentsEmployeesMapForEmployeeLevel);
        }

        // Get the employee list for the employee's department
        String deptName = e.getDepartment();
        List<Employee> employeesListForEmployeeLevelAndDepartment = departmentsEmployeesMapForEmployeeLevel.get(deptName);
        if (Objects.isNull(employeesListForEmployeeLevelAndDepartment)) {
            employeesListForEmployeeLevelAndDepartment = new ArrayList<>();
            departmentsEmployeesMapForEmployeeLevel.put(deptName, employeesListForEmployeeLevelAndDepartment);
        }

        // add the employee to its department list
        employeesListForEmployeeLevelAndDepartment.add(e);
    }

    return dataMap;
}

Also as mentioned in the comments, this can be further simplified using Map.getOrDefault.

private static Map<Integer, HashMap<String, List<Employee>>> createDataMap2(List<Employee> employeeList) {
    Map<Integer, HashMap<String, List<Employee>>> dataMap = new HashMap<>();

    for(Employee employee: employeeList){
        int level = Integer.parseInt(employee.getLevel());
        String deptName = employee.getDepartment();

        // get the list of department employees for level
        HashMap<String, List<Employee>> departmentsEmployeesMapForEmployeeLevel = 
                dataMap.getOrDefault(level, new HashMap<>());
        // get the list of employees for level and department
        List<Employee> employeesListForEmployeeLevelAndDepartment = 
                departmentsEmployeesMapForEmployeeLevel.getOrDefault(deptName, new ArrayList<>());

        // add employee to its department for its level
        employeesListForEmployeeLevelAndDepartment.add(employee);
        // set new employee list to the employee's department
        departmentsEmployeesMapForEmployeeLevel.put(deptName, employeesListForEmployeeLevelAndDepartment);
        // set the updated department to the employee's level
        dataMap.put(level, departmentsEmployeesMapForEmployeeLevel);
    }

    return dataMap;
}
Bentaye
  • 9,403
  • 5
  • 32
  • 45