0

There are two code code 1:

if(isApplicable() || isGood()) {
   //something
}

private boolean isApplicable() {
}

private boolean isGood() {
}

Code 2:

boolean applicable = isApplicable();
boolean good = isGood();

if(applicable || good) {
   //something
}

private boolean isApplicable() {
}

private boolean isGood() {
}

Which of the the approach is good java practice ? To me code1 seams more clean and code 2 seams to have extra code. code2 can make remote debugging easy.

saurabh agarwal
  • 2,124
  • 4
  • 24
  • 46
  • 3
    Short: The first. If you're not going to use the variable anywhere else, don't create it to begin with. There is an exact duplicate for this question, too, just can't find it atm. – domsson Jul 18 '17 at 09:27
  • Less code means less space for bugs. – Steve Smith Jul 18 '17 at 09:28
  • The first, practice to make the code short. It is easier for debugging purposes. – UmarZaii Jul 18 '17 at 09:29
  • The first. But it is still an opinion. – MC Emperor Jul 18 '17 at 09:31
  • If the method call more than one time, it is bettor to go for the second option else first option is good – janith1024 Jul 18 '17 at 10:04
  • Another related question: https://stackoverflow.com/questions/43369494/is-there-a-purpose-why-java-developers-are-doing-this – domsson Jul 18 '17 at 10:14
  • Possible duplicate of [Local variables before return statements, does it matter?](https://stackoverflow.com/questions/31733811/local-variables-before-return-statements-does-it-matter) – domsson Jul 18 '17 at 10:14

1 Answers1

1

To generalise your question, you're asking about the two forms:

// local variable form
Foo foo = methodReturningFoo();
Bar bar = methodTakingFoo(foo);

// inlined form
Bar bar = methodTakingFoo(methodReturningFoo());

Most modern IDEs have a shortcut to refactor between these at a keystroke: "inline" and "extract local variable". The fact that both refactorings exist is an indicator that both are appropriate, in different circumstances.

Inlining to a single statement makes the code more compact and sometimes more readable. You can see everything that's happening without having to read up to find out where a variable was set.

Here's a good candidate for inlining:

String name = customer.getName();
String greeting = createGreeting(name);

// ... becomes ...
String greeting = createGreeting(customer.getName());

Extracting a local variable turns what may be a long statement into two (or more) shorter statements. It may also allow you to re-use a value rather than calculate it twice.

Here's an example where we just break a statement into smaller chunks.

String greeting = createGreeting(greetingFactory.get(customer.getTitle()), customer.getName());

// ... becomes ...
Title title = customer.getTitle();
String name = customer.getName();
String greeting = createGreeting(greetingFactory.get(title), name));

... here's an example where we reuse a calculated value.

// doing the work twice
CustomerCategory category = findCategory(totalOrderValues(
     customer.getOrders(currentMonth)));
List<Promotion> eligiblePromotions = findEligiblePromotions(totalOrderValues(
     customer.getOrders(currentMonth)));

// ... becomes ...
BigInteger totalOrderValues = totalOrderValues(
    customer.getOrders(currentMonth))
CustomerCategory category = findCategory(totalOrderValues);
List<Promotion> eligiblePromotions = findEligiblePromotions(totalOrderValues);

Generally, prefer the inlined version, until you see that the line is too long and complicated. Then extract a local variable (or extract a method) to make it neater. If it makes sense to store a value to avoid repeating an expensive calculation, then do so.

slim
  • 40,215
  • 13
  • 94
  • 127