-3

I wonder if has any good technic can make the code below more clean.

final Optional<Long> historyHours1;
final Optional<Double> historyHours2;
final Optional<Float> historyHours3;

if (parameters.isPresent()) {
  historyHours1 = // some logic
  historyHours2 = // some logic
  historyHours3 = // some logic
} else {
  historyHours1 = Optional.empty();
  historyHours2 = Optional.empty();
  historyHours3 = Optional.empty();
}

want to keep the variables as final, and use some technic to replace the if-else. considered to use a method to do the calculation and return an object which contains the there variables, but to combines the there variables into an object, is not expected.

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
jaxcy
  • 1
  • The code doesn't need refactoring. Replacing the `if ... else` will only make it harder to read, and possibly less efficient. – Stephen C Jul 05 '21 at 04:35

2 Answers2

0

You probably don’t want to use Optional for your three final variables. Optional, OptionalDouble and OptionalLong are often great for return values that may not be there. For local variables in a method that is 20 lines or shorter they don’t really buy you anything. According to my taste the same goes for final, but I acknowledge that opinions are more diverse here.

My suggestion would be (given that you want final variables):

final Long historyHours1 = parameters.map(p -> /* some logic */).orElse(null):
final Double historyHours2 = parameters.map(p -> /* some logic */).orElse(null):
final Float historyHours3 = parameters.map(p -> /* some logic */).orElse(null):

The cost of this rewrite is that we’re testing three times whether your parameters are present. That‘s probably fine, but you will need to weigh that yourself.

In case you insist on Optional, the same idea works:

final OptionalLong historyHours1 = parameters.stream()
                                             .mapToLong(p -> /* some logic */)
                                             .findAny();

Similar for the other variables. Use OptionalDouble and OptionalLong. And Optional<Float> since there is no OptionalFloat class. Even though for unboxing into OptionalLong or OptionalDouble we need to go via a stream as shown in the code line.

Depending on what you will be using the historyHoursX variables for it is also possible that a rewrite involving that is possible, also a rewrite that exploits parameters being an Optional (when I have guessed that correctly). So you may want to show us more of your existing code.

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
-2

use switch might help when you might add more else conditions.

Tangtang
  • 1
  • 2
  • 1
    But [@jaxcy](https://stackoverflow.com/users/16380436) asks how to rewrite code without a conditional. The switch will not help here. – ilyazub Jul 05 '21 at 09:10
  • Please read the actual question before trying to answer. – khelwood Jul 05 '21 at 09:53
  • the question is to make the code cleaner and replace the if-else, i don't see there is ask how to rewrite code without a conditional. – Tangtang Jul 06 '21 at 09:11