1

I apologize in advance if this question has been asked, but I have been struggling to figure this out for a few hours now (googling everything I can) and really hope someone could help.

I am trying to collect two String values, which I am trying to get from a source, in to a single String, and return null if these values do not exist (source.getA returns null).

So far, I have come up with this code:

if (source.getA() != null && source.getB() != null) {
    target.setAAndB(Stream.of(source.getA(), source.getB())
            .map(String::valueOf)
            .collect(Collectors.joining(" ")));
} else {
    target.setAandB(null);
}

How do I turn this in to a nice looking chain? I know there is a way, but I feel like I am missing a trick here.

EDIT:

target.setAandB(
        Optional.of(
                Stream.of(source.getA(), source.getB())
                        .map(String::valueOf)
                        .collect(Collectors.joining(" "))
        ).orElse(null)
);

Made a bit of progress, but here the orElse will not work, because the collector still returns a string with two null strings combined.

EDIT 2:

Thank you all for your answers, I found them very valuable and some valuable things to consider.

edudins
  • 36
  • 5
  • 2
    I'd like to understand what getA and getB functions are. And why you are using streams for just 2 values. – Luca Scarcia Aug 30 '22 at 17:57
  • So basically you're creating a Stream and Collector just in order to concatenate two Strings. You're clearly **misusing** the Stream API. – Alexander Ivanchenko Aug 30 '22 at 18:01
  • @AlexanderIvanchenko but do you know, how I can turn this in to a chain without the `if`? – edudins Aug 30 '22 at 18:17
  • @e-dudins It implies you don't care if you're **abusing** Streams and Optional or not? Did hear about ***clean-coding***? Optional and Streams should not be used to hide conditions. – Alexander Ivanchenko Aug 30 '22 at 18:21
  • @AlexanderIvanchenko no, I care a lot about this stuff. – edudins Aug 30 '22 at 18:23
  • @e-dudins `I care a lot` - Then you should not try to replace conditional with something which is not meant for such purpose. – Alexander Ivanchenko Aug 30 '22 at 18:34
  • @e-dudins: to check for not null you should use *.filter(s -> s!=null)* Still I don't understand why you use map(String::valueOf). don't getA and getB already return a string? – Luca Scarcia Aug 30 '22 at 18:42
  • 1
    `target.setAandB(source.getA() != null && source.getB() != null? source.getA() + " " + source.getB(): null);` – Holger Aug 31 '22 at 07:50

2 Answers2

0

Ternary operator can be used

target.setAandB(Objects.isNull(source.getA()) && Objects.isNull(source.getB()) ? null 
           : Stream.of(source.getA(), source.getB())
                  .map(String::valueOf).collect(Collectors.joining(" ")));
Santanu
  • 691
  • 3
  • 5
0

There are no benefits in hiding conditional logic by using Optional.

See the answer by @StuartMarks, Java and OpenJDK developer.

The primary use of Optional is as follows:

Optional is intended to provide a limited mechanism for library method return types where there is a clear need to represent "no result," and where using null for that is overwhelmingly likely to cause errors.

A typical code smell is, instead of the code using method chaining to handle an Optional returned from some method, it creates an Optional from something that's nullable, in order to chain methods and avoid conditionals.

It is not justifiable to create Optional to avoid null-checks. It obscures the logic and makes your code harder to read.

And using Streams just in order to concatenate only two Strings is clearly an overkill. We have enough tools for that, apart from streams.

That said, your code might be written like that:

if (source.getA() != null && source.getB() != null) {
    target.setAandB(source.getA() + " " + source.getB());
} else {
    target.setAandB(null);
}

Alternatively, you can introduce a method responsible for generating this String:

public String getAB() {
    
    return source.getA() == null || source.getB() == null ? null :
        source.getA() + " " + source.getB();
}

Note: that if getA() and getB() are not plain getters, and source object does something behind the scenes to fetch the data, you need to store both into local variables instead of fetching twice.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46