HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Collecting data from Vector to Map

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mapcollectingfromdatavector

Problem

I wrote the following piece of code and want to hear your opinion, in this snippet I have a vector called aggregateFeaturesForML which has elements of Class with 3 fields: sourceip, key, value. what I want is to collect all the key-value pairs that have the same sourceip and form a nice histogram (for each IP the keys are unique), for that purpose I collect the key-value pairs in a map called histogram and then use its toString function to print a {key=value, key=value} formation. I store the histograms inside another map called aggregator.

Any comments on the readability, writing manner, other stuff will be appritiated.

Definition:

private HashMap > aggregator;
protected Vector aggregateFeaturesForML = new Vector(); //Single result has 3 fields: IP, key, value


Code:

String previousIp = aggregateFeaturesForML.get(0).getSourceip();
        if(!aggregator.containsKey(previousIp))
        {
            aggregator.put(previousIp, new HashMap());
        }

        HashMap histogram = new HashMap(); 
        for(int iterator=0;iterator mapForIP = aggregator.get(previousIp); 
                mapForIP.put(key, histogram.toString());
                aggregator.put(previousIp, mapForIP);

                if(!aggregator.containsKey(ip))
                {
                    aggregator.put(ip, new HashMap());
                }

                histogram.clear();
                previousIp = ip;
            }

            histogram.put(sr.getKey(), sr.getValue());
        }
        HashMap mapForIP = aggregator.get(previousIp);
        mapForIP.put(key, histogram.toString());
        aggregator.put(previousIp, mapForIP);

Solution


  • Beware of hard-coding the first element of a list like aggregateFeaturesForML.get(0). This can throw an exception is aggregateFeaturesForML is empty so you should check for that first.



-
Do not compare String with == (or !=). In the following

if(ip != previousIp)


you are comparing the Strings ip and previousIp with !=. Strings are compared using their equals method, so you should have instead:

if(!ip.equals(previousIp))


-
Prefer to program against interfaces. Instead of

HashMap mapForIP = aggregator.get(previousIp);


use:

Map mapForIP = aggregator.get(previousIp);


As a side-note, if you are using Java 8, your code could be written a lot more simply using the Stream API, that directly provides a way to group elements with a classifier, using the groupingBy collector. In this case, when two elements are classified the same way, they are collected into a Map with the toMap collector.

Map> aggregator =
    aggregateFeaturesForML.stream()
                          .collect(Collectors.groupingBy(
                              SingleResult::getSourceip,
                              Collectors.toMap(SingleResult::getKey, SingleResult::getValue)
                          ));

Code Snippets

if(ip != previousIp)
if(!ip.equals(previousIp))
HashMap<String, String> mapForIP = aggregator.get(previousIp);
Map<String, String> mapForIP = aggregator.get(previousIp);
Map<String, Map<String, String>> aggregator =
    aggregateFeaturesForML.stream()
                          .collect(Collectors.groupingBy(
                              SingleResult::getSourceip,
                              Collectors.toMap(SingleResult::getKey, SingleResult::getValue)
                          ));

Context

StackExchange Code Review Q#118609, answer score: 5

Revisions (0)

No revisions yet.