Yes Virginia, even more WTFs are real



  • Java programmers may be confused by this real, production code extract till they follow the code step-by-step.

    private static Map<Integer, String> cdwEvents = new HashMap<Integer, String>();
    ...
    public static String getCdwEventName(int code) {
        if (cdwEvents == null || cdwEvents.size() == 0)
            initiliaseCdwEvents();
        int key = 0;
        for (Iterator<Integer> i = cdwEvents.keySet().iterator(); i.hasNext();) {
            key = i.next();
            if (key == code) {
                return (String) cdwEvents.get(key);
            }
        }
        return null;
    }

    They will wonder why the algorithm is manually iterating over the lookup keys of an in-memory table to find a record when a standard library class can already do this. Then they will realise the record retrieval line is actually using the standard library lookup, making all the code around it pointless. That is, the function can be reduced to the following and even that's got redundant checks.

    public static String getCdwEventName(int code) {
        if (cdwEvents == null || cdwEvents.size() == 0)
            initiliaseCdwEvents();
        return cdwEvents.get(code);
    }

    Then they will say "WTF!"

    I have to maintain code like this. Weep for me.



  •  I've seen that one so many times that I'm more likely to exclaim "Not again!" before I fix it.



  •  Why do you complain? this code work. All you have to do is a BIIIIIIIIIIIG replace, no?



  •  Maybe they used to use a list of int-string-pairs, but later changed it to a HashMap for optimization purposes.

     Or the original author of the code was afraid that calling get on a nonexistent value could throw an exception.



  • @geocities said:

     Maybe they used to use a list of int-string-pairs, but later changed it to a HashMap for optimization purposes.

     Or the original author of the code was afraid that calling get on a nonexistent value could throw an exception.

    If only some mechanism existed that could, say, try to catch exceptions... If only...



  • @geocities said:

     Maybe they used to use a list of int-string-pairs, but later changed it to a HashMap for optimization purposes.

     Or the original author of the code was afraid that calling get on a nonexistent value could throw an exception.


    Starting as a list of int/string pairs for micro-optimization purposes would be a possible explanation but failing to remove redundant code when trying to micro-optimise becomes a WTF. Alas, checking the history, the code has always used a HashMap. As for, non-existent keys, the Map interface is defined as returning null if the key can't be found, so no exception is thrown.



  • Someone needs to be fired for not getting the concept of a HASHMAP.



  • @hoodaticus said:

    Someone needs to be fired for not getting the concept of a HASHMAP.


    He resigned, which is why I'm maintaining this code. I am told his wife was worse.



  • @pjt33 said:

     I've seen that one so many times that I'm more likely to exclaim "Not again!" before I fix it.

    I wonder if there is some kind of semantic patch tool for java.



  • @spamcourt said:

    @pjt33 said:

     I've seen that one so many times that I'm more likely to exclaim "Not again!" before I fix it.

    I wonder if there is some kind of semantic patch tool for java.

     

    "Semantic patch tool".  That's warmachine's job title.

     



  • The programmer was just ensuring portability to architectures where the int type has multiple representations of the same number. For example, a 36-bit architecture would have 4 unused padding bits in a word storing a 32-bit int value. Or with ones-complement integers, there is both a positive and a negative zero. Since different representations would have different hash codes with high probability, you must pass the same representation of the key to HashMap.get as was used to HashMap.put. Iterating through the keys to find the one with the same numeric value is an easy way to enforce this.

    (ok, not realistic in Java, but if this was C code it could happen...)



  • @Goplat said:

    The programmer was just ensuring portability to architectures where the int type has multiple representations of the same number. For example, a 36-bit architecture would have 4 unused padding bits in a word storing a 32-bit int value. Or with ones-complement integers, there is both a positive and a negative zero. Since different representations would have different hash codes with high probability, you must pass the same representation of the key to HashMap.get as was used to HashMap.put. Iterating through the keys to find the one with the same numeric value is an easy way to enforce this.

    (ok, not realistic in Java, but if this was C code it could happen...)

    I know you're just joking, but it could only happen in C if the internal implementation of the library function used a memcmp rather than just a bog-standard operator== to compare the keys, and that could happen just as easily in Java as in C - which is to say, not unless the standard library was written by an idiot.




  •  So what the programmer really needs is to be educated about TreeMap?



  • @Goplat said:

    ok, not realistic in Java

    LTFY

    Side note: TRWTF is that containsKey(), get() and remove() of a Map<Key,Val> accept a parameter of type Object instead of Key. Seriously, if I have a Map<String,String>, am I asking too much if I expect my compiler to warn about someMap.containsKey/get/remove(someObjectWhichIsNotAStringButWhichHasAStringFieldWhoseAccessorIForgotToCall) being always false/null/noop, respectively? </rant>



  • @warmachine said:

    He resigned, which is why I'm maintaining this code. I am told his wife was worse.

     

    He resigned because he didn't get the concept of his wife?

     



  • If
    @Goplat said:

    [...] you must pass the same representation of the key to HashMap.get as was used to HashMap.put

    it's not a hash map, but a steaming pile of shit. All real hash maps have requirement on the hash function that equivalent values have the same hash.



  • @fatbull said:

    Side note: TRWTF is that containsKey(), get() and remove() of a Map<Key,Val> accept a parameter of type Object instead of Key. Seriously, if I have a Map<String,String>, am I asking too much if I expect my compiler to warn about someMap.containsKey/get/remove(someObjectWhichIsNotAStringButWhichHasAStringFieldWhoseAccessorIForgotToCall) being always false/null/noop, respectively? </rant>


    I was among those arguing the same point back when generics were in the process of being added. Unfortunately the relevant Sun engineer's position was that since you can write equals methods which make two objects of different classes equal, that would introduce a false restriction; he couldn't be persuaded that a) if you did that then you were an idiot; b) if you had control of both classes to do that, you could create a new marker interface for them and use that as the key type.



  • @Bulb said:

    All real hash maps have requirement on the hash function that equivalent values have the same hash.
    That's actually the requirement for hash codes generally, not just for maps.



  • @warmachine said:

    He resigned, which is why I'm maintaining this code. I am told his wife was worse.
    Then I guess you should be grateful that you don't have to maintain his wife.


Log in to reply