Ternary If Hiding Duplicate Code

I realized an unintentional side effect of deciding to not use the ternary if (a ? b : c) – there’s a bunch of code that we don’t duplicate that we otherwise probably would have. In particular, when working with views there’s a very common pattern to convert between a Shape and a Rectangle1:

Rectangle bounds = shape instanceof Rectange ? (Rectangle)shape : shape.getBounds();

The reason for this is that getBounds() on a Rectangle will create a new Rectangle instance so if you happen to be getting the bounds a few thousand times every time you go to paint the text pane, you generate an awful lot of useless rectangles. Since every shape passed in is in fact a Rectangle, you can eliminate all of that junk without risking a ClassCastException if at some point you do get a different type of shape.

Now there’s nothing about the ternary if that means you can’t refactor this into a common utility method, but because it makes it all fit on one line it doesn’t feel like this is duplicate code – after all you’d be replacing one line with a one line method call so what’s the difference. When you break it out into a full blown if with braces and all, you suddenly see the encapsulated business logic which one day might go and change. Refactoring it into a common method suddenly seems like a really good idea.

That said, this isn’t why I don’t use ternary if statements – I just don’t like the look of them. Purely an aesthetic thing, if I wind up working with a code base that does use them I’m quite happy to go along with it. If I get to decide though, I tend to find the longer form far more readable. YYMV.

1 – Shape for the most part being a completely useless base class↩