8

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) {
    ...
  }
}

8 Answers 8

19

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.

1
  • Thanks - This is actually already my preferred approach; I just wanted some validation from the programmer community.
    – Adamski
    Sep 17, 2009 at 11:17
13

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...

3

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

1
  • 2
    Nothing, I love Lisp, it just happens to have lots of parenthesis, that's all. Sep 17, 2009 at 16:41
3

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.

1

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.

1

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
0

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).

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.