Better null-management try-catch with Java 8

Question

Given the following code:

StaticPOIDataType response = null;
try {
  final JAXBContext jc = JAXBContext.newInstance(ObjectFactory.class);
  final Unmarshaller unmarshaller = jc.createUnmarshaller();
  unmarshaller.setSchema(getSchema());

  response =((JAXBElement<StaticPOIDataType>) unmarshaller.unmarshal(cpoGatewayURL)).getValue();

} catch (JAXBException e) {
  e.printStackTrace();
} finally {
  return response;
}

I heard of a new Java 8-feature to express this in a much more elegant way by avoiding null-initializiation. But I don't know what to search for.

maybe: https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html — ΦXocę 웃 Пepeúpa ツ, Jun 06 '17 at 12:30
in Java 7 you have try-with-resource that you can found here [Am I using the Java 7 try-with-resources correctly](https://stackoverflow.com/questions/17650970/am-i-using-the-java-7-try-with-resources-correctly) that the only update in `try` that I know — AxelH, Jun 06 '17 at 12:30
`return` in `finally` is very "dangerous" : https://stackoverflow.com/questions/15225819/try-catch-finally-return-clarification — luk2302, Jun 06 '17 at 12:30
just drop the `finally`, drop the `response`, write `return ...` where you currently set a value to `response`. And after the entire try-catch, return `null`. — luk2302, Jun 06 '17 at 12:31
@luk2302 It is more the case in a try-finally, not in a try-catch-finally. — davidxxx, Jun 06 '17 at 12:32
@davidxxx The catch does not really matter, if you `return` in the `try` and return in the `finally` that still causes unexpected behaviour. — luk2302, Jun 06 '17 at 12:33
try-with-ressources doesn't help because there are no resources which have to be closed — barracuda317, Jun 06 '17 at 12:34
@davidxxx and the next developer might add second a `return`. Just because the code is working now is no excuse for writing code that is error-prone in the future. — luk2302, Jun 06 '17 at 12:35
@luk2302 Like you, I prefer avoiding returning in a finally statement. It is less readable and may be error prone. But saying that it is "very dangerous" is probably exaggerated. — davidxxx, Jun 06 '17 at 12:36
Literally one of the first results in Google when you type `resource management try-catch` is the thing you need. I think this question was asked without doing any prior work or research. — M. Prokhorov, Jun 06 '17 at 12:48
@M.Prokhorov i don't see what my example has to do with resources — barracuda317, Jun 06 '17 at 12:52
You asking about resource management, which is indicated by title of your question. — M. Prokhorov, Jun 06 '17 at 12:53
@M.Prokhorov resource don't necessary mean `closable`... try-with-resource only works with those. This is just the title that is not correctly defined — AxelH, Jun 06 '17 at 12:57
@AxelH, almost any resource interface can be represented by composing `AutoCloseable` and some other interface. Even in cases of resource pools where `dispose()` means "return to pool", the pool implementation can provide `AutoCloseable` wrapper/facade on which `close()` will return underlying resource to parent pool. — M. Prokhorov, Jun 06 '17 at 13:01
@M.Prokhorov "_almost any resource interface can be represented_" but they are not. So you can't declared a resource that is not implementing it. But that's not the point. read the question and you will see that this is not what OP want ;) . — AxelH, Jun 06 '17 at 13:05

slim · Answer 1 · 2017-06-06T13:22:08.473

What your code does is:

invoke some methods
if it throws an exception, log (or whatever) the error and return null
otherwise return what it returns

The cleanest way to do this -- and this is not new -- is:

try {
    return someMethod(...);
} catch (SomeException e) {
    logOrWhatever(e);
    return null;
}

Some people object to multiple returns -- but in a short block like this, it's the clearest way (and if your block isn't this short, do the extract-method refactoring until it is).

The alternative with just one return is longer and messier, and involves declaring a mutable variable in a wider scope than necessary:

Foo response = null;
try {
   response = someMethod(...);
} catch (SomeException e) {
   logOrWhatever(e);
}
return response;

It has more clutter. Think about how much reading you have to do to answer the question "What does this method return if someMethod() throws an exception?". In the first example it's right there in the catch block. In the second you have to follow the code up through all possible state changes to response.
response can't be declared final -- I don't think you should use final everywhere, but I do think it's good to have as many variables as possibly that could be declared final.
The scope of response is outside the try block. The tidier method keeps that data in a narrower scope, which is always good.

By putting the return in a finally block, your code does something else -- if any other type of exception is thrown, null will be returned without any logging or other exception-handling. This is unlikely to be a good idea -- it just makes bugs hard to diagnose: Try-catch-finally-return clarification

However, more generally, returning null is something you should try to avoid. Your question title mentions "null-management", but it's you that has introduced a null to proceedings. If you didn't choose to return null, you wouldn't have to deal with its problems:

If null means "something went wrong", don't do that. Handle the exception higher up (if you like, re-throw it as a different exception type)
If the semantics of the method are "This might return a response, or there may not be a response", consider:
- Making the method return Optional<Response> -- now the method signature is honest
- Using the "Null object" pattern. That is, instead of returning null return new EmptyResponse()

This way more of your code can be much simpler and cleaner, because it doesn't have to be full of if(x==null) guards.

@AxelH `Map` was designed 19 years ago. We continue to use it because it's there. But we shouldn't repeat its mistakes. If designed today it would probably return `Optional`. — slim, Jun 06 '17 at 13:33
Well `Optional` is the opposite ;) It's to recent, at the office, we are still running Java 6... ;-) To end on something constructive, this [post](https://stackoverflow.com/q/1274792/4391450) as some good argumentation for what is a valid/invalid usage of null. I just disagree to those who said "Never to use NULL" ;) — AxelH, Jun 06 '17 at 13:40
@AlexH Java 6 is 11 years old. But it's very easy to roll your own `Optional`, or to use the null object pattern, or to use a lib like Guava (albeit an ancient version thereof) — slim, Jun 06 '17 at 13:55
@AlexH fine, but the question does not include anything about being constrained by historical decisions. — slim, Jun 06 '17 at 15:15
I never said it was a problem... You are the one that kept argumenting, so it change the subject slightly.. And come on, it **axelh** :( — AxelH, Jun 06 '17 at 15:26

Rohan S Raju · Answer 2 · 2017-06-06T12:39:49.657

1

What you are looking is for optional. here is the below code enjoy

try {
        final JAXBContext jc = JAXBContext.newInstance(ObjectFactory.class);
        final Unmarshaller unmarshaller = jc.createUnmarshaller();
        unmarshaller.setSchema(getSchema());

        return Optional.ofNullable((JAXBElement<StaticPOIDataType>) unmarshaller.unmarshal(cpoGatewayURL)).getValue());

    } catch (JAXBException e) {
        e.printStackTrace();
    }

    return Optional.empty();

edited Jun 06 '17 at 12:39

answered Jun 06 '17 at 12:35

Rohan S Raju

426
3
7
20

It could be that, but I personnaly prefer the "null" version as you only have one return statement in the method. – AxelH Jun 06 '17 at 12:37
there are two return statements return Optional.ofNullable((JAXBElement) unmarshaller.unmarshal(cpoGatewayURL)).getValue()); return Optional.empty(); – Rohan S Raju Jun 06 '17 at 12:38
That's my problem ;) I know about that logic, but I found messy to have more than one "return" statement in a method. – AxelH Jun 06 '17 at 12:40
Then change the method signature to throw JAXBException exception and then convert into you application Exception and handle it. then you will have one return – Rohan S Raju Jun 06 '17 at 12:43
i get: reason: no instance(s) of type variable(s) T exist so that Optional conforms to StaticPOIDataType – barracuda317 Jun 06 '17 at 12:44
@AxelH :) enjoying helping :) – Rohan S Raju Jun 06 '17 at 12:49
@barracuda317 if you do this you need to change the signature of your method to `public Optional myMethod(...)`. Callers obviously have to be changed accordingly: http://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html – slim Jun 06 '17 at 13:38

score -2 · Answer 3 · answered Jun 06 '17 at 12:34

-2

I think you are looking for Optional in Java 8,

Please find a this documentation with nice samples.

https://dzone.com/articles/java-8-optional-avoid-null-and

answered Jun 06 '17 at 12:34

sanoj mathew

91
8

Better null-management try-catch with Java 8

3 Answers3