views:

498

answers:

5

EDIT: The entire code and database creation script can be found from http://gitorious.org/scheator . The database script is in Schema/.

I have the following Java code:

A LinkedHashMap defined in an abstract class as

LinkedHashMap<Object, Data> list;

A descendant class that initializes this list like this:

list = new LinkedHashMap<Integer, Data>();

I add items like this:

    String id = rs.getString(FIELDS[0]);
    String name = rs.getString(FIELDS[1]);
    Data team = new Data(Integer.parseInt(id.trim()), name);
    list.put(id, team);

Now when I do this:

    System.err.println("delete() count: " + list.size());

    System.err.println("delete() key value " + key);
    Data obj;
    obj = (Data)list.remove(key);
    deletedList.put(key, obj);
    System.err.println("delete() count: " + list.size());

Nothing is removed from the list, i.e. the first and last prints print the same size(). The key is also correct (I have verified there is an item by that id).

However, and this is my question, if I add the values like this:

    Integer id = rs.getInt(FIELDS[0]);
    String name = rs.getString(FIELDS[1]);
    Data team = new Data(id, name);
    list.put(id, team);

The code works! Shouldn't parseInt() produce a similar key to getInt()? Why does the second version work but the first doesn't? I spent a good hour debugging this until I found the reason and I still can't figure out the reason.

A: 

My guess is that int the second case you cast it explicitly into an Integer

Integer id = rs.getInt(FIELDS[0]);

while on the first case it remains an int

Integer.parseInt(id.trim())

from the javadoc of parseInt

static int  parseInt(String s) 
Parses the string argument as a signed decimal integer.
Eric
Yes..., but it will get auto-boxed into Integer when it's added to the map. And the Integer hashCode and equals methods just use the int primitive value. So it shouldn't matter.
Matthew Flaschen
+2  A: 

There should be no difference, but there are a number of things that are not clear from your example:

  • deletedList does not refer to the list object
  • the records in your database that are being used are the same in both cases (perhaps in the first a different int is being used that is already in the Map)

Autoboxing may also be complicating the issue. Replace Integer id in the second sample with int id to pass the same arguments to your Data constructor.

Maybe you could post up the complete code such that we can reproduce the scenario accurately?


Update

You are using String values as keys in your original code. You then have an Object key in your remove(key) method, so I expect you are passing an Integer to the method at this point. A String will not match an Integer as a key, which explains why your remove was not working.

If you use generics to specify your HashMap (LinkedHashMap<Integer, Team> instead of <Object, Team>) this kind of error can't happen - the compiler will say something like

The method put(Integer, Object) in the type HashMap<Integer,Object> is not applicable for the arguments (String, String)

Brabster
The complete code is hundreds of lines of code. Just the two classes I mentioned are > 300 lines and they would be far from a runnable example. You can get the entire code in it's current infancy from http://gitorious.org/scheator . Commit 2b34bde has the non-working code, while commit c64daed3 has been fixed. The files in question are in src/ScheatorDb: AbstractDb.java and Teams.java.
Makis
Oh, and you still won't be able to run without a MySQL database. I haven't added the SQL commands to create the database yet to the repository.
Makis
Looking into it. Don't really have time but can't resist a puzzle!
Brabster
I added the database schema in case you want to run it. Warning: even though I have coded for 20 odd years this is my first Java application and under development to top it off, so expect poor code!
Makis
Agreed, using generic to store Object is a perversion. It's like not using generics at all.
Lluis Martinez
I want to be able to use other key types than just Integers for descendants of the DbObject class. It is initialized as <Integer, Data> in the Team class, though.
Makis
You can use wildcards to refine down from <Object> to a subset of types, but I suspect if you really want a map with arbitrary objects as keys you will just have to be very careful about this kind of type problem. If it was me, I'd be looking at my design to see if I'd got problems with the abstraction between my persistence layer and my application's domain model.
Brabster
A: 

If I were you I would inspect the contents of the LinkedHashMap using a debugger, before and after your put and before and after your remove. Step into the remove() method (the source code is part of the JDK) and see what it is doing. Odds are your code is not adding or removing the object correctly. It's hard to see here because the code sample is incomplete.

As for rs.getInt() and Integer.parseInt(), the first is database-vendor specific (I assume rs is a ResultSet), and thus they may not have the same behaviour. However, once the Integer key is created (you can verify this with your debugger) it should be equivalent for HashMap or LinkedHashMap purposes. But your code sample complicates things further; you are using rs.getString() and then Integer.parseInt(). While I would be surprised if this happened, it's possible that the database driver is formatting the id column into a string that confuses parseInt(). To me it's far more readable to just do rs.getInt().

Mr. Shiny and New
+2  A: 

First example:

String id = rs.getString(FIELDS[0]);

Second example:

Integer id = rs.getInt(FIELDS[0]);

I can't say for sure since I can't see the rest of the code, but if the key variable is an Integer in this call:

obj = (Data)list.remove(key);

then the remove will only work if the object was put into the map using an Integer and that is why it is only working when the id is integer when you call the put method. The String "123" does not equal the integer 123.

Also I am assuming that you just missed a line in your first example but there was no call to list.put(id, team) but that could also be the source of your problems

Yanamon
Yes, I accidentally left out the put() from the first example. It's exactly the same as in the second.
Makis
+1  A: 

Yanamon is right. It's pretty clear when you look at the diff:

             while (rs.next()) {
-                String id = rs.getString(FIELDS[0]);
+                Integer id = rs.getInt(FIELDS[0]);
                 String name = rs.getString(FIELDS[1]);
-                Data team = new Data(Integer.parseInt(id.trim()), name);
+                Data team = new Data(id, name);
                 list.put(id, team);

Note that in the original version, an int (auto-boxed to Integer) is being passed into the Data constructor. But id, which is being putted, is still a String.

Matthew Flaschen
And this is the correct answer. It's ridiculous how long you can look at the code and not see a simple problem like this. I already fixed id to be Integer and fetching it with getInt() which is a much better way of doing this in the first place.
Makis