saurabh agarwal
saurabh agarwal

Reputation: 2174

Best practice for variable saving

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.

Upvotes: 0

Views: 247

Answers (1)

slim
slim

Reputation: 41203

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.

Upvotes: 1

Related Questions