Hashtable lookup "workaround"
-
So I'm at a code review, and I see something like the following:
CustomKey key = ...;
CustomValue value = null;
// Find the value corresponding to the key.
Iterator it = myHashMap.iterator();
while(it.hasNext())
{
Map.Entry entry = it.next();
if ( entry.getKey().equals(key) )
{
value = (CustomValue)entry.getValue();
break;
}
}
So I point to the code and ask "how come you don't just call myHashMap.get(key)?"
The answer I got back was "I tried it like that, and it wasn't returning my value, so I put the loop in and it worked."
WTF? A hashtable lookup isn't working so, instead of trying to figure out why, you defeat the whole purpose of using a hashtable by looping through the damn thing to hunt for your key?
As it turned out, the hashtable lookup wasn't working because MyCustomKey went something like this:
class MyCustomKey
{
public boolean equals(Object other)
{
// Compare the fields...
}
// Some other methods, none of which is getHashCode
}
-
You should use Findbugs or PMD. Both of these tools would have caught the source of the problem (overriding equals() without overriding hashcode()).
-
@WayneCollins said:
So I point to the code and ask "how come you don't just call myHashMap.get(key)?"
The answer I got back was "I tried it like that, and it wasn't returning my value, so I put the loop in and it worked."
WTF? A hashtable lookup isn't working so, instead of trying to figure out why, you defeat the whole purpose of using a hashtable by looping through the damn thing to hunt for your key?
this kind of shit happens all the time. Today in a code review I was talking to someone about building test point names like:
1. testing foo when bar is even.
2. testing foo when bar is odd.
3. testing foo when bar is prime.
4. testing foo when baz is even.
5. testing foo when baz is odd.
6. testing foo when baz is prime.
7. testing foo when moop is even.
8. testing foo when moop is odd.
9. testing foo when moop is prime.and they were defined:
addTestPoint("testing foo when bar is even")
addTestPoint("testing foo when bar is odd")
...so when I said why not
foreach variable in (bar, baz, moop)
foreach value in (even, odd, prime)
addTestPoint("testing foo when " + variable + " is " + variable);they said that they tried that and it wasn't working.
-
@tster said:
foreach variable in (bar, baz, moop)
foreach value in (even, odd, prime)
addTestPoint("testing foo when " + variable + " is " + variable);they said that they tried that and it wasn't working.
Now, that's probably because they should have used "value" instead of "variable" as the second .. err.. variable.