Local variable assignment to avoid multiple casts

Question

There was recently a question asking whether it was a good idea in Java to assign the results of calling a getter to a local variable to avoid multiple calls to the same accessor. I can't find the original post but the consensus seemed to be that this generally isn't necessary as Hotspot will optimise away the method call overhead anyway.

However, what is the view on employing this technique to avoid multiple casts? At the moment I'm faced with a choice between:

if (a instanceof Foo) {
  // Cast once and assign to local variable.
  Foo foo = (Foo)a;

  if (foo.getB() == 1 && foo.getC() == 2) {
    ...
  }
}

OR

if (a instanceof Foo) {
  // Cast twice making code compact but possibly less readable.
  // Also, is there an overhead in multiple casts?
  if (((Foo)a).getB() == 1 && ((Foo)a).getC() == 2) {
    ...
  }
}

Answer 1

I prefer the first, as it looks more clean. The second is starting to look like Lisp. But it's just a personal opinion.

Answer 2

I prefer creating the local variable rather than always casting because of readability issues. Code readability, for me (or other developers working on the same code), is an important issue.

Worrying about performance at this stage strikes me as an example of the "premature optimization" pattern.

Answer 3

Absolutely a good idea as it improves clarity. I would say that applies for avoiding multiple accessor calls too - it's a good idea for clarity not performance reasons.

Answer 4

Definitely go with the first. The performance difference is likely to be irrelevant, but the readability is definitely improved.

In addition to removing the casts, it also means you get to use a different name - after all, you now know more about this variable, so it may well make sense to give it a more specific name. That's not always the case, but it can be. The "introduce local variable for the sake of naming a value" refactoring technique is an underappreciated one, even without the casts...

Answer 5

I'd say the important thing is not to optimise prematurely. If there is an overhead to casting, it's likely to be so small that it would be effectively unnoticeable in practice. Unless this code snippet formed the majority of your application's CPU time, I don't think you'd see any measurable performance difference between the two.

Consequently I'd also go for the first option as it looks cleaner and is easier to understand and modify - much more important than executing a couple of clock cycles faster in 99.99% of situations.

Answer 6

I prefer the first option, for two reasons

1. The necessary parantheses make the code cludgy and hard to read
2. There's possibly a (small) overhead in the cast

Answer 7

I prefer the first, not just for code readability, but for runtime debugging sake (also for the original example - if you put the result of the getter in a local, you see that value rather than having to trace into the getter the first time).

Answer 8

I agree with people that say the first version is preferable, but I would like to add that if at all possible—and it almost always is possible—you should avoid casting in the first place. Not for any performance reasons, but just for the extra check on code correctness.