current community

your communities

more stack exchange communities

Stack Exchange
tour help
stack overflow careers
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote -1 down vote favorite

I'm working on a homework assignment (CS 106A, Handout #8 ). It is a trip itinerary program where the user selects an origin and gets a list of potential destinations.

I'm stuck on the data storage section where I'm reading a text file (2nd page of pdf) and storing it. I chose to store data in a HashMap<String, ArrayList<String>>; where the key would be the trip origin and the value would be an ArrayList with all destinations available from the given origin.

This is the code I have so far (https://gist.github.com/cch5ng/8139898)

//imports

import acm.program.*;
import acm.io.*;
import acm.graphics.*;
import java.util.*;
import java.util.HashMap;
import java.util.Iterator;
import java.awt.event.*;
import java.awt.Color;
import java.awt.TextField;
import java.io.*;
import java.nio.file.*;
import java.nio.charset.*;
import javax.swing.*;
import javax.swing.event.MouseInputAdapter;

public class handout8 extends ConsoleProgram {

public void init() {
    //read in flights.txt data
    origin_old = "";
    Path file = Paths.get("flights.txt");
    Charset charset = Charset.forName("US-ASCII");
    try (BufferedReader reader = Files.newBufferedReader(file, charset)) {
        String line = null;
        while ((line = reader.readLine()) != null) {
            origin_new = "";
            //System.out.println("origin_old: " + origin_old + ".");
            dest = "";
            //destinations.clear();
            int k = 0;
            while (line.charAt(k) != 0 && k < line.length() - 1 && line.charAt(k + 1) != 45) { //45 for -
                origin_new += Character.toString(line.charAt(k));
                k++;
            }
            if (line.length() > 1) {
                System.out.println("k: " + k);
                System.out.println("origin_old: " + origin_old + ".");
                System.out.println("origin new: " + origin_new + ".");
                if (k != 0) {
                    dest = line.substring(k + 3);
                }
                //issue here where all the keys end up with the same destinations array list
                //I know I can correctly get the number of origin cities, so maybe I need to hardcode the array list names somehow?
                if (origin_old.equals(origin_new)) {
                    destinations.add(dest);
                    System.out.println("added dest");
                } else {
                    destinations.clear();
                    System.out.println("cleared dest");
                    destinations.add(dest);
                }
                System.out.println("dest: " + dest);
                System.out.println("num destinations: " + destinations.size());
                //error in logic adding data to hashmap
                //issue here where all the keys end up with the same destinations array list
                //ArrayList<String> destin_old = new ArrayList<String>();
                //destin_old = destinations;
                if (!origin_new.equals("")) {
                    mp_destinations.put(origin_new, destinations);
                }

                origin_old = origin_new;
                printDest(origin_new);
            }
        }
    } catch (IOException e) {
        println("IOException: " + e);
    }
    printKeys();
    printDest("Denver");
    printDest("New York");
    //printDest("San Francisco");
    //printDest("San Jose");
    //printDest("Honolulu");
    //printDest("Anchorage");
}

private void printKeys() {
    Iterator it = mp_destinations.keySet().iterator();
    System.out.println("Origins: ");
    while (it.hasNext()) {
        System.out.println(it.next());
    }
}

private void printDest(String start) {
    ArrayList<String> dest2 = mp_destinations.get(start);
    if (dest2.size() > 1) {
        Iterator i = dest2.iterator();
        System.out.println("Destinations for " + start + ": ");
        while (i.hasNext()) {
            System.out.println(i.next());
        }
    } else {
        System.out.println("Destination for " + start + ": " + dest2.get(0));
    }
}
//ivars
private ArrayList<String> destinations = new ArrayList<String>();
private HashMap<String, ArrayList<String>> mp_destinations = new HashMap<String, ArrayList<String>>();
private String origin_old, origin_new, dest;
}

(the issue seems to be around lines 50-67) While looping through the lines of the text file, it appears the program is defining the keys and the respective ArrayList contents appropriately. However when I check the values of the HashMap at the very end, all the returned values are only appropriate for the last key added. I guess all the keys appear to be mapped to the same ArrayList.

Do I need to hardcode a unique ArrayList per number of keys? How do I define and maintain different ArrayLists (dynamically) per key (assuming I don't know the # of keys before running the program)? TIA.

share|improve this question
6  
Post the relevant code here, no one wants to follow links. –  Sotirios Delimanolis Dec 27 '13 at 5:39
    
(Off-topic): Can you use Guava? If so, consider a MultiMap to simplify dealing with Map<K, List<V>> –  Greg Kopff Dec 27 '13 at 5:41
1  
Questions concerning problems with code you've written must describe the specific problem — and include valid code to reproduce it — in the question itself. See SSCCE.org for guidance. –  Christian Dec 27 '13 at 5:41
    
You need a unique ArrayList per key. or every key has the same set of values (which you clearly don't want). –  Elliott Frisch Dec 27 '13 at 5:48

2 Answers 2

up vote 1 down vote accepted

You need to assign a brand new ArrayList to destinations for each key of the map. Otherwise the keys will all be mapping to the same object.

Replace this line:

destinations.clear();

with

destinations = new ArrayList<String>();
share|improve this answer
    
Thank you, that worked. –  cchung Dec 27 '13 at 5:58
    
notice that with the given code, you will have an error the second time a destination is added to existing origin, as it will replace the first destination with the second, instead of adding the second to it. –  tzafrir Dec 28 '13 at 9:39
up vote 0 down vote

you seem to misuse the HashMap put method. whenever you call map.put you replace the current value stored with the new value given. consider the following code snippet

ArrayList<String> destinations = new ArrayList<String>();
//add new destination
destinations.add("dest1");
destinations.add("dest2");

if (!origin_new.equals("")) {
    //before adding to map obtain the current list of destinations
    ArrayList<String> currentDestinations = mp_destinations.put(origin_new);
    if (currentDestinations == null)
        //if the origin doesn't exists, create the destinations list
        currentDestinations = new ArrayList<String>();

    //add all the new destinations to the list
    currentDestinations.addAll(destinations);

    //store the modified list in your map
    mp_destinations.put(origin_new, currentDestinations);                   
}
share|improve this answer
    
tzafrir, for each line of the text file I am reading, I'm checking whether the key (city of origin) is the same as the previous line's key. If so, I am updating the ArrayList of destinations. Then, if so, I do want to replace the mapping for that key with the updated ArrayList. You can argue that my program could be redesigned for efficiency but that was a design choice and works as expected. See: if (origin_old.equals(origin_new)) { destinations.add(dest);} –  cchung Dec 29 '13 at 2:35
    
@cchung I was not aware that the list you are reading is sorted. if the list is not sorted (e.g origin paris, origin london, origin paris) your code breaks. also, notice that in the case you mentioned if (origin_old.equals(origin_new)) { destinations.add(dest);} there is no need to replace the value stored in the map, as you are actually modifying it when adding the second destination –  tzafrir Dec 30 '13 at 9:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.