0

I am trying to build a program where I add running times to a specific location. I am then storing the location and time in a hashmap. As I get the running time, I add it to a LinkedList, and then try to put the newly-updated LinkedList at the hashmap's Key value. However, once I move onto a new location, the running times don't stay with their designed location, so all locations end up having the same running time. I'm not quite sure what I am doing wrong. Thank you for any help.

Example data: Location A: 45 sec, 43 secs, 36 secs Location B: 51 sec, 39 secs

Incorrect Output: Location A: 39 secs, 51 secs Location B: 39 secs, 51 secs

Correct Output: Location A: 36 secs, 43 secs, 45 secs Location B: 39 secs, 51 secs

    HashMap h = new HashMap();
    LinkedList times = new LinkedList();
    LinkedList newTimes = new LinkedList();


public static void addInformation(HashMap h, LinkedList times, LinkedList    
newTimes) {

   String location = scanner.next();
   Double time = scanner.nextDouble();

   if (h.containsKey(location)){
        for (int i = 0; i < newTimes.size(); i++){
            times.add(newTimes.get(i));
        }
        times.add(time);
        getFastTime(times);
        h.get(location).add(location, times); // cannot resolve add method 
    }else{
        newTimes.clear();
        newTimes.add(time);
        getFastTime(newTimes);
        h.put(location, newTimes);
    }
}
public static void printInformation(HashMap h) {
    Set keySet = h.keySet();  
    for ( Object locationName : keySet) {
        //Use the key to get each value. Repeat for each key.
        System.out.println("Location =" + locationName + " Time =" + 
    h.get(locationName));
    }
}

public static void getFastTime(LinkedList times){
   times.sort(null);
}
Michelle
  • 365
  • 1
  • 4
  • 18
  • 2
    While you can certainly roll your own, both Appachie Commons and Google provide a `MultiMap` implementation which allows you to store multiple values at the same key. Using one of these will likely greatly simplify your implementation. See here: http://stackoverflow.com/questions/14925329/how-do-i-import-multimap-for-java for how to get them. – aruisdante Feb 18 '15 at 23:11

3 Answers3

4

The issue is the fact that Java passes by reference. You are not creating new lists for the different locations so the same list is being used for all entries in the map. You should read up on this because it is a fundamental aspect of Java.

Next, your collections should be parameterised. You dont need the times and newTimes lists. Also use List rather than LinkedList in the map. Something like so:

HashMap<String, List<Double>> map = new HashMap<>();

And do the same in the method definitions. There are a number of other issues, such as the printInformation method which assumes the Objects are Strings without even casting them. Input is not validated. What if the input is malformed? This should be considered. Also, variables should be named better.

Something like this should work (untested. you will also have to look at the print method to make it work with a list):

HashMap<String, List<Double>> map = new HashMap<>();

public static void addInformation(HashMap<String, List<Double>> map) {
    //input should be validated here
    String location = scanner.next();
    Double time = scanner.nextDouble();

    List<Double> timesInMap = map.get(location);
    if (timesInMap != null){
        timesInMap.add(time);
        timesInMap.sort(null);
    }else{
        timesInMap = new ArrayList<Double>();
        timesInMap.add(time);
        map.put(location, timesInMap);
    }
}
public static void printInformation(HashMap<String, List<Double>> map) {
    Set<String> keySet = map.keySet();  
    for (String locationName : keySet) {
        //Use the key to get each value. Repeat for each key.
        System.out.println("Location =" + locationName + " Time =" + 
                map.get(locationName));
    }
}
Brandon
  • 317
  • 1
  • 10
  • You're never `put`ting the newly created array list on the map. And the right way to do this is either use an existing `Multimap` implementation or create one of your own and then let your program use it via its operations. – fps Feb 18 '15 at 23:51
  • You're right, i forgot that line. Updated my answer. I disagree that the right way is to use a third party library. A list at each key will suffice. See a related question here: http://stackoverflow.com/questions/8229473/hashmap-one-key-multiple-values. – Brandon Feb 18 '15 at 23:54
  • OK, I accept that it's not incorrect to do it this way, manually. Maybe for a project from college this is OK. But for production systems, why reinventing the wheel? Libraries out there are efficient, well-tested and have nice interfaces. Actually, there's a comment below the accepted answer of the question you've linked that says exactly this. But, as you said, you can do this by your own. However, if you plan to use this same functionality many times, you should encapsulate it in a class, i.e. `MyOwnMultimap`. – fps Feb 19 '15 at 00:02
  • 1
    I agree. For a larger system i would also look into using a custom `Multimap` of some sort. What I meant is that for an extremely simple project like this, no reason why you have to use any third-party libraries. – Brandon Feb 19 '15 at 00:09
1

In Java, when you pass a parameter or get an object, you actually handle a reference on this object. Mostly, the collections are mutable (even if you can create immutable too), meaning you can modify a collection in place. It's preferable to use the Generics to support strong typing, and that should solve your problem of add method not resolved.

E.G., the hashmap should be declared as :

HashMap<String, List<Double>> h = new HashMap<>();

On the left hand side, it's only declaring what the hashmap shall contain, no List<Double> is instanciated yet.

I'm not sure you need to globaly declare the times and newTimes lists. The time values will be stored in different List<Double> for each entry in the HashMap<>. The addInformation function could have the following logic :

// Add one (location, time) scanned from input
public static void addInformation(HashMap<String, List<Double> locations,
                                  Scanner scanner) {
   String location = scanner.next();
   Double time = scanner.nextDouble();

   List<Double> times = locations.get(location);
   if (times == null) {
      // this is a new location, create the linkedlist
      // and put it in the hashmap
      times = new LinkedList<Double>();
      locations.put(location, times);
   }
   // add the time to the linkedlist
   times.add(time);
}

In the above implementation, the times are ordered by insertion (first element has been inserted first). If you care to have this time values always sorted, you can sort the list each time you add a new (location, time), i.e. add the following line just after times.add(time) :

Collections.sort(times);

The sort modifies the list - you can have more details in the java doc Collections.sort.

Another option is to use a TreeSet<Double> instead of the LinkedList<Double>. The elements will be maintained sorted, but no duplicate time is allowed.

If you need to keep duplicates of time values and a sorted collection, then you have the TreeMultiset that does just that.

Also, be aware that the order either with Collection.sort or with a sorted collection like TreeSet can be controlled with a Comparator. For instance you may want to have the times in decreasing order (natural order will be increasing for Double).

T.Gounelle
  • 5,953
  • 1
  • 22
  • 32
0

try this code, you have your linklists defined global and shared by all location keys,

HashMap h = new HashMap();
public static void addInformation(HashMap h, LinkedList times, LinkedList    
newTimes) {

   String location = scanner.next();
   Double time = scanner.nextDouble();

   if (h.containsKey(location)){
        LinkedList times = h.get(location);
        times.add(time);
        getFastTime(times);

    }else{
        LinkedList newTimes = new LinkedList();
        newTimes.add(time);
        getFastTime(newTimes);
        h.put(location, newTimes);
    }
}
faljbour
  • 1,367
  • 2
  • 8
  • 15
  • Hmm I'm still getting the error. I'm initiating the LinkedList in my main function, and then passing it to my addInformation method. If I define them locally, they reset every time I call this method (which I do frequently.) When I try to define the LinkedList times, I still get an error when trying to get. Sorry if I'm not understanding your code. – Michelle Feb 19 '15 at 00:20
  • I just updated the code, I removed h.get(location).add(location, times); from the code when the location exists in the Hashmap. You instantiate the LinkList once and add it to the Hashmap, then you access it every time you need to update it and add to it. – faljbour Feb 19 '15 at 00:35