how to get rid of checks for null and if-else if signature in this code?

Question

I get complex object response. It look like this:

    class ComplexObject{
        private PartOne partOne;
        private PartTwo partTwo;
        private PartThree partThree;
    }

And I need process this response. Now I do it like this:

    if (partOne!= null) {
        processOne(partOne);
    } else if (partTwo != null) {
        processTwo(partTwo);
    } else if (partThree != null) {
        processThree(partThree);
    }

But it looks bad. If I could influence the external service, I would add enum Status.PART_ONE, Status.PART_TWO, Status.PART_THREE but I can't do it.

How do I rewrite this code to make it cleaner?

I imagine all 3 parts are different, right? so you do need a different method to process each — dquijada, Dec 12 '18 at 10:04
use an optional: Optional partOne. then do if(partOne.isPresent()) — ItFreak, Dec 12 '18 at 10:04
Possible duplicate of [Avoiding != null statements](https://stackoverflow.com/questions/271526/avoiding-null-statements) — Akceptor, Dec 12 '18 at 10:04
If only one Part in the ComplexObject will be non-null at a time, then the best thing to do is abstract out Part from the ComplexObject with an abstract method process() and have three concrete classes for Part, say PartOne, PartTwo and PartThree. — Vinodh, Dec 13 '18 at 04:51

davidxxx · Accepted Answer · 2018-12-12T11:09:07.890

2

If PartOne, PartTwo, PartThree don't make part of a base class, you could not take advantage of polymorphism.
In this case, your actual way is an acceptable trade off.

I would just write that in this way :

if (partOne!= null) processOne(partOne);
else if (partTwo != null) processTwo(partTwo);
else if (partThree != null) processThree(partThree);

edited Dec 12 '18 at 11:09

answered Dec 12 '18 at 10:12

davidxxx

125,838
23
214
215

unfortunately these are completely different classes. The first part is a successful response with details. the second part is an error with the details (status, description). The third part is also an error (another kind of error). ugly api. To be honest, there is an `enum` - but the third party always returns the same type in any case (success, error or something else) – ip696 Dec 12 '18 at 10:22
in `Optional` case how can I interrupt algorithm if first part not null and I do not need continue? – ip696 Dec 12 '18 at 10:32
In this case I would stick to your actual code. 3 null check is not a drama. In fact a possibility to improve this code would be to change your client service class that returns the 3 objects. Make them return an Optional of the objects. In this way this would make your actual code clearer. – davidxxx Dec 12 '18 at 10:32
MyComplexObject - JAXB object(autogenerated). in processing method if partTwo and partOne is not null - what happened? – ip696 Dec 12 '18 at 10:44
Not sure to understand. – davidxxx Dec 12 '18 at 10:49
1

Doesn't this solution behave slightly differently than the one in the question? In the question, at most one of the process methods is ever called; here, up to three might be called. – OhleC Dec 12 '18 at 10:51
@ohlec Completely right. I didn't see it. I removed this refactoring part that will finally be helpless. – davidxxx Dec 12 '18 at 11:02
@ip696 as Ohlec noticed : I forgot a point : the processings are exclusive in the conditional statements. So I removed the refactoring possibility. – davidxxx Dec 12 '18 at 11:03
@davidxxx Yes, I just meant what wrote Ohlec. apparently and really have to leave so. – ip696 Dec 12 '18 at 11:06
@davidxxx your polymorphic solution still suffers from this – OhleC Dec 12 '18 at 11:07
Ohlec, you denuded my whole answer. I didn't read the question correctly. So thanks ! – davidxxx Dec 12 '18 at 11:09

SamHoque · Answer 2 · 2018-12-12T10:33:07.753

0

How about using java.util.Optional provided by Java 8 like @ItFreak Suggested in the comments, Your code would look like

Optional.ofNullable(partOne).ifPresent(p -> processOne(p));
Optional.ofNullable(partTwo).ifPresent(p -> processTwo(partTwo));
Optional.ofNullable(partThree).ifPresent(p -> processThree(partThree));

If you are using Java 9 or higher you can do this

Optional.ofNullable(partOne).ifPresentOrElse(p -> processOne(partOne),
        () -> Optional.ofNullable(partTwo).ifPresentOrElse(p -> processTwo(partTwo), 
                () ->  Optional.ofNullable(partThree).ifPresent(p -> processThree(partThree))));

edited Dec 12 '18 at 10:33

answered Dec 12 '18 at 10:11

SamHoque

2,978
2
13
43

1) `Optional.of()` may throw NullPointerException if null is passed as parameter 2) `ifPresent()` returns a `Consumer`. Something like that is required : `Optional.ofNullable(partOne).ifPresent(p -> processOne(p));` – davidxxx Dec 12 '18 at 10:18
What do you see `Empty.of()`? @davidxxx – SamHoque Dec 12 '18 at 10:18
I have changed it to `ofNullable` Thanks :) – SamHoque Dec 12 '18 at 10:19
I don't have an excuse for the Consumer part, If i used an IDE I would have spot this error but I just wrote the code directly on the SO Post :P – SamHoque Dec 12 '18 at 10:21
If values are passed for PartOne, PartTwo and PartThree all then it'll process all three which isn't happening in the code provided in the question – Aditya Narayan Dixit Dec 12 '18 at 10:23
and how will I interrupt the execution if the first part is not null and I do not need to check the another? – ip696 Dec 12 '18 at 10:24
`ifPresentOrElse` – SamHoque Dec 12 '18 at 10:27
`ifPresentOrElse` what? – ip696 Dec 12 '18 at 10:30
1

@SamzSakerz Do you really think that this option is better than the original?))) – ip696 Dec 12 '18 at 10:46
@ip696 What do you mean by "better" It's personal opinion, You should use the code that makes you happy :D – SamHoque Dec 12 '18 at 10:47
Well, in general, I wanted advice on how to make my code cleaner) and not to rewrite it to an even more cumbersome form – ip696 Dec 12 '18 at 10:50
It's up to you, You can see which one looks cleaner in your workspace to you. – SamHoque Dec 12 '18 at 10:51

how to get rid of checks for null and if-else if signature in this code?

2 Answers2