CODEWITHSUNDEEP
javaif-statementoption-type
Javi
Javi

Reputation: 41

Simplify many if checks with Optional

if (reg[0] != null && reg[0].trim().length() > 0) {
   orderData.setCity(reg[0]);
}
if (reg[1] != null && reg[1].trim().length() > 0) {
   orderData.setCountry(reg[1]);
}
if (reg[2] != null && reg[2].trim().length() > 0) {
   orderData.setObjectType(reg[2]);
}
if (reg[3] != null && reg[3].trim().length() > 0) {
   orderData.setChannel(reg[3]);
}

Can I simplify this with Optional, or other Java 8 features?

Upvotes: 1

Views: 136

Answers (4)

Tarun
Tarun

Reputation: 723

Optional.ofNullable(reg[0]).filter(val -> val.trim().length() > 0).ifPresent(orderData::setCity);
Optional.ofNullable(reg[1]).filter(val -> val.trim().length() > 0).ifPresent(orderData::setCountry);
Optional.ofNullable(reg[2]).filter(val -> val.trim().length() > 0).ifPresent(orderData::setObjectType);
Optional.ofNullable(reg[3]).filter(val -> val.trim().length() > 0).ifPresent(orderData::setChannel);

Upvotes: 0

rzwitserloot
rzwitserloot

Reputation: 102902

Optional is probably not going to help you here.

The first thing to think of, is sentinel values. Here you are clearly indicating that you think that, semantically, reg[0] being null is equivalent to reg[0] being the empty string (or even a string with only spaces in it). That is supposed to be extraordinary. null is not a standin for 'nothing' or 'empty'. null is supposed to be a standin for 'There is no value'. The best way to do this, is to find the place that generates reg[0], and ensure that the proper semantic meaning (an empty string) is set up here. Then this code can simply be: if (!reg[0].isEmpty()) orderData.setCity(reg[0]); - so much cleaner.

That is, of course, not always possible. For example, if reg is coming down the pipe from a library or other code that simply isn't under your control, or it is an object created by, say, a JSON demarshaller.

In that case I generally would advise creating an explicit step to convert an object that is 'not clean' (has nulls even when semantically that's supposed to be an empty string) into a clean one.

If that is not feasible or not possible, well, working with an 'unclean' object is never going to be particularly pretty, style wise. A helper method would help a lot, here:

normalizeThenSetIfNonBlank(reg[0], orderData::setCity);
normalizeThenSetIfNonBlank(reg[1], orderData::setCountry);

private void normalizeThenSetIfNonBlank(String in, Consumer<String> target) {
    if (in == null) return;
    in = in.trim();
    if (in.isEmpty()) return;
    target.accept(in);
}

Upvotes: 2

Makoto
Makoto

Reputation: 106430

Optional would be the wrong tool here.

Leverage StringUtils.isNotBlank instead.

if (StringUtils.isNotBlank(reg[0])) {
   orderData.setCity(reg[0]);
}

You still have to write each value out since they map to different fields, but this will clean it up and allow you to use a more conventional way to check for a blank string.

Upvotes: 3

Naman
Naman

Reputation: 31878

If you were to really look for an implementation involving Optional here, the method I had suggested in comments could be implemented to return one

private Optional<String> validInput(String input) {
    return Optional.ofNullable(input)
                   .filter(in -> in.trim().length() > 0);
}

then this can be used as 

validInput(reg[0]).ifPresent(city -> orderData.setCity(city));
validInput(reg[1]).ifPresent(country -> orderData.setCountry(country));
... and the likes

Upvotes: 4

Related Questions