ArrayList to HashMap function behaving oddly

Question

So I am working on the question of the best way of finding the two most common elements in an array list.

My method is to turn the whole thing to a hashmap, then see which one is the greatest. Because I like hashmaps. They seemed like a good way to do it and I wasn't able to come up with a better solution.

Except that I am getting an error. Which is where you come in (= !

public static String[] findTwo(ArrayList<String> param) {   
    Map<String, Integer> counter = new HashMap<String, Integer>();

    for (int i = 0; i < param.size(); i++) {
        //param.get(i) is name of inserted object
        if (counter.get(i) == null) {
            counter.put(param.get(i), 1);
            System.out.println(counter.get(i) + "<-- should be 1"); // <-- erroneous part!
        } else {
            counter.put(param.get(i), counter.get(i)+1);
            System.out.println("elsing");
        }
        System.out.println(counter);
    }
    return null;
}

This code prints

null<-- should be 1
{HIHI=1}
null<-- should be 1
{HIHI=1}
null<-- should be 1
{HIHI=1}
null<-- should be 1
{HIHI=1}
null<-- should be 1
{yoyo=1, HIHI=1}
null<-- should be 1
{yoyo=1, HIHI=1}
null<-- should be 1
{yoyo=1, nono=1, HIHI=1}
null<-- should be 1
{yoyo=1, nono=1, froyo=1, HIHI=1}

Which is completely wrong!

It is saying that the value is null after I insert a 1. I am not sure why that is? =(

Ahhh thank you all!

Can anyone help me figure out what the time cost is?

Dude. Not `counter.get(i)`. `counter.get(param.get(i))`. at the top of your loop, do something like `String nextString = param.get(i)`, and you won't get confused later on — iluxa, Nov 05 '13 at 04:02
What IDE or compiler are you using? `counter.get(i)` should raise an error or warning, since i is not a String. And that error would help lead you to the fix. — user949300, Nov 05 '13 at 04:07

score 4 · Accepted Answer · edited May 23 '17 at 12:29

4

counter.get(i) == null should be counter.get(param.get(i))

The fact counter(i) compiles is because Map#get receives an Object and i gets autoboxed from int to Integer (which is an Object).

A better approach would be using enhanced for loop iteration over your List<String> param:

for(String parameter : param) {
    if (!counter.containsKey(parameter)) {
        //logic key is not present...
    } else {
        //logic when key is present...
    }
}

Also, start programming oriented to interfaces instead of class implementations directly. Use List backed by ArrayList:

public static String[] findTwo(List<String> param) {
    //...
}

More info on this:

What does it mean to "program to an interface"?

edited May 23 '17 at 12:29

Community

1
1

answered Nov 05 '13 at 04:02

Luiggi Mendoza

85,076
16
154
332

How does `counter.get(i)` even compile, since counter is defined as a `Map`? Should have been a compiler error or warning. – user949300 Nov 05 '13 at 04:06
1

@user949300 because [`Map#get`](http://docs.oracle.com/javase/7/docs/api/java/util/Map.html#get(java.lang.Object)) receives an `Object` and `i` gets autoboxed from `int` to `Integer` (which is an `Object`). – Luiggi Mendoza Nov 05 '13 at 04:07
Hmm, seems like `Map.put()` checks the params vs. the generics, but not `Map.get()`. Interesting obscure knowledge. Anybody know why? And yet another example of autoboxing strangeness. – user949300 Nov 05 '13 at 04:14
@user949300 I won't say its obscure since it is documented and published online. – Luiggi Mendoza Nov 05 '13 at 04:15
At the very least, it fails the principle of least astonishment. I just read the comments and links from http://stackoverflow.com/questions/857420/what-are-the-reasons-why-map-getobject-key-is-not-fully-generic and it's pretty obscure! – user949300 Nov 05 '13 at 04:22

score 2 · Answer 2 · answered Nov 05 '13 at 04:06

Here is the problem.

Change your following line of code

System.out.println(counter.get(i) + "<-- should be 1");

To

System.out.println(counter.get(param.get(i)) + "<-- should be 1");

Reason is, you are making param.get(i) as key to your counter map counter.put(param.get(i), 1); and getting it using i. So it is returning NULL which is true as there is no value mapped at key i.

ArrayList to HashMap function behaving oddly

2 Answers2