Java, the wrong way



  • I just found this gem in a web application that I am cleaning up.  Everybody here thought the world of her while she was here.  She may have had charisma, but she needs to stay away from a keyboard.  This section of code is one of the many-many non-functioning parts of the application.

     

                double chkTotal = 0.0;
                TreeSet<String> sortedKeys = new TreeSet<String>();
                sortedKeys.addAll(bucketHM.keySet());
                Object key[] = sortedKeys.toArray();


                //Object key[] = bucketHM.keySet().toArray();
                for (int i=0;i < bucketHM.size();i++)
                {
                    ...snip HSSF bs...

                    chkTotal = new BigDecimal(chkTotal).add(new BigDecimal(((Double)bucketHM.get((String)key[i])).doubleValue())).doubleValue();

                }

    And just in case you were wondering, yes this project is in Java 1.5 (see TreeSet<String>).


  •  Brillant!



  • @TwelveBaud said:

     Brillant!


    I was going to say "paid by the line", but your post seems more accurate. Never accuse someone of malice when it's more likely that they're just stupid.



  •  Using an Iterator could easily take as many lines, and being paid by the line wouldn't account for the failure to use the type system properly.



  • No, there was no paid by the line stuff going on here.  This person was just hacking things together and had a poor understanding of Java. She is using a Genericed TreeSet<String> to add all of the elements from the non-generic HashMap keys.  Alright, now we have a Set of elements that is type checked.  So instead of adding them to a String array we use an Object array that we need to cast later...  Just brillant (as stated above).  

    This entire app is full of hacks and atrocious code.  I'm sure that I will be posting more wonderful code as it comes up.



  • @pjt33 said:

     Using an Iterator could easily take as many lines, and being paid by the line wouldn't account for the failure to use the type system properly.

     

    Maybe paid my nesting depth?



  • I find a supple pair of breasts and tight ass go a long way to ensuring your job security when employed in the engineering field. Your boss might also be more likely to buy a pair of 24" monitors for you.



  • For those who aren't familiar with Java 1.5 the following will suffice to perform the logic required of the code snippet in the OP:

    		double chkTotal = 0.0;
    		for(Double value: bucketHM.values())
    		{
    			//...snip HSSF bs...
    			chkTotal += value.doubleValue();
    		}
    
    

    Of course, if you need access to the key values for the absent "HSSF bs" logic then the next snippet will be a better fit:

    		for(Map.Entry entry: bucketHM.entrySet())
    		{
    			String key = (String)entry.getKey();
    			Double value = (Double)entry.getValue();
    			//...snip HSSF bs...
    			chkTotal += value.doubleValue();
    		}
    


  •  Well, it might be better to sort the Doubles by absolute value before adding them to reduce loss of significance.



  • @DeepThought said:

    For those who aren't familiar with Java 1.5 the following will suffice to perform the logic required of the code snippet in the OP:

    		double chkTotal = 0.0;
    		for(Double value: bucketHM.values())
    		{
    			//...snip HSSF bs...
    			chkTotal += value.doubleValue();
    		}
    

    Of course, if you need access to the key values for the absent "HSSF bs" logic then the next snippet will be a better fit:

    		for(Map.Entry entry: bucketHM.entrySet())
    		{
    			String key = (String)entry.getKey();
    			Double value = (Double)entry.getValue();
    			//...snip HSSF bs...
    			chkTotal += value.doubleValue();
    		}
    

     

    Absolutely not.  You should never use a double to calculate currency in Java.  Maybe it wasn't obvious by looking at the code and variable names, so it's excusable. 



  • For people who don't read those nine linked words above:

    The WTF "avoided" is using inexact floating point to represent exact money. TRWTF is that they go through all that work to not use floating point, and then do so anyway.
    And no, I didn't spot it in that first post.


  • @DeepThought said:

    Of course, if you need access to the key values for the absent "HSSF bs" logic then the next snippet will be a better fit:

    for(Map.Entry<String, Double> entry: bucketHM.entrySet())
    {
    	String key = entry.getKey();
    	Double value = entry.getValue();
    	//...snip HSSF bs...
    	chkTotal += value.doubleValue();
    }
    

    FTFY. If you're going to use generics, at least use them consistently...



  • @Fred Foobar said:

    @DeepThought said:
    Of course, if you need access to the key values for the absent "HSSF bs" logic then the next snippet will be a better fit:

    for(Map.Entry<String, Double> entry: bucketHM.entrySet())
    {
    	String key = entry.getKey();
    	Double value = entry.getValue();
    	//...snip HSSF bs...
    	chkTotal += value.doubleValue();
    }
    

    FTFY. If you're going to use generics, at least use them consistently...
     

     

    You are correct. Started Java development back in the mid-to-late nineties and still haven't gotten used to the new-fangled generics.  

     Thanks for the feedback.


Log in to reply