« What inspires you? | Main | Things you should positively not do while driving »

Bad code annoyance with Java #234840

Gratuitous casting to get around bad design.

I refactored code today that did something like this:

// Imagine this method returns an interface
// iStuff which is implemented by an object called Stuff. 
// 
public static iStuff getStuff(int key) { 
   iStuff rval = new Stuff(key); 
   return rval;
} 

public void doSomeStuff(int key) { 
   // Note, this was just cast to an object!
   //  
   Stuff stuff = (Stuff) getStuff(key); 
   stuff.doSomeChange();      
}

Sorry, this is the best way I could duplicate what the code did without actually copying the code. Point is: Instead of changing the design to reflect the fact that the interface did not have a method needed.. someone went and cast an interface to an object. How horrible! Yet legal. Also very dangerous as it will break if someone changes the underlying implementation of the getStuff() method.

Posted by kasia on March 3, 2003 10:51 PM |

Comments

look on the bright side, at least you're not stuck with coworkers who subscribe to the 'error handling via printf' method of programming...

i seriously saw the following code today:

try {
// do something that could throw an exception
} catch (const exception & exp) {
printf ("exception: %s\n", exp.what());
}

i mean that's just fucking great. i can tell what went wrong if i'm looking at the program while it's running, but there's NO FUCKING WAY to programatically tell what went wrong (as this function only returns 1 for success and -1 for failure).

and it's not like this is a one time thing, he does this ALL THE TIME!

but i'm not bitter...

Posted by: garrett | March 3, 2003 11:13 PM

Garrett - I've encountered a similar situation. Not pretty at all. I feel for you...

Posted by: Rich Dougherty | March 4, 2003 12:18 AM

Our entire framework requires this. We have a swing application that requires components to be created in a factory. The factory returns an interface that's pretty much useless. Everyone just casts it to what it really is:

OurComponent comp = ComponentFactory.getComponent("key");
((JTextField)comp).setEnabled(false);

These aren't morons either. They just don't care.

Posted by: Bryan Young | March 4, 2003 10:15 AM

Oh, don't get me wrong, there are places where it's appropriate. I've written factory objects that return generic objects before.. but in this particular case it was used to avoid adding a call to the interface or, rather, change the design to get around the fact that the object was supposed to be read-only! (Interface only had getters, not setters).

What you would do in terms of a factory is assure that it's safe and a bad-cast call isn't doable.

Posted by: kasia | March 4, 2003 10:18 AM