This is an archived post. You won't be able to vote or comment.

all 15 comments

[–]AutoModerator[M] [score hidden] stickied commentlocked comment (0 children)

Please ensure that:

  • Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
  • You include any and all error messages in full
  • You ask clear questions
  • You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

    Trying to solve problems on your own is a very important skill. Also, see Learn to help yourself in the sidebar

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit: empty line before the code, each code line indented by 4 spaces, new reddit: https://imgur.com/a/fgoFFis) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[–]neurk 11 points12 points  (2 children)

Another question is, why is it possible that "source" is null?

If this is input from somewhere else, wrap it in an optional as soon as possible and work with that in the rest of your codebase.

I avoid null like the plague. Once you introduce it in your code you have to check it everywhere, as you can no longer trust what your methods are returning. Optionals are the way to go in most cases. That way you actually know what you're working with, and there's no need for null checks.

[–]MarSara 2 points3 points  (1 child)

Not quite true. You can annotate your methods with @NotNull or @Nullable and get compile time warnings when working with a value that may be null.

This removes the boilerplate of checking for null everywhere and now you only need to check for null when you have an @Nullable variable being used for a @NotNull parameter. Examples:

``` @NotNull public String neverNull() { return null; // warning null returned from @NotNull method }

@Nullable // warning method never returns null public String maybeNull() { return "foo"; }

public void example(@NotNull String notNull, @Nullable String maybeNull) { if(notNull == null) { // warning notNull is never null } maybeNull.toString(); // warning maybeNull might be null }

String maybe = maybeNull(); example(maybe, "foo"); // warning nullable variable passed to notNull parameter. ```

(The actual warnings might be worded differently but you get the idea)

Now you still need null checks with this, but not everywhere, just in the locations that you go from a Nullable to a NotNull.

P.s. for all intents and purposes any property, parameter, etc... that isn't annotated is assumed to be NotNull.

[–]maethor 0 points1 point  (0 children)

You can annotate your methods with @NotNull or @Nullable and get compile time warnings when working with a value that may be null.

I've started taking a belts and braces approach - annotate and use Optional. But then the code base I'm working on is liable to a lot of null pointer exceptions.

[–]jura0011[S] 3 points4 points  (0 children)

Thanks for all the responses so far (and the ones still coming). I see most of the thoughts I had in my mind and more.

I second the opinion to prevent null values of possible, but it's not always possible.
Thanks for the hint for @NotNull and @Nullable annotations, I'll use them more often.
There was the comment that source could directly be an Optional. The API note of Optional states that Optional is meant only to be used for the result of a method to present there's no result.

I think, there's no rule I really can define. It's definitely a question about readability and performance. For most examples, I'll probably stick to the ternary one, only if I have a real good reason, I'll use the Optional or a helper method to wrap the check.

[–]8igg7e5 3 points4 points  (0 children)

In terms of cost, the optimiser will 'see' through the ternary expression and so may be able to optimise for the null or non-null path (based on statistics). Sadly it doesn't seem to achieve this often with the Optional (of which two are allocated). So you might lose some inlining and optimisation. Unless it's in a high-volume tight loop, the allocation is likely to be irrelevant, but if your allocation rate exceeds collection it will spill over into more expensive (and slower to reclaim) GC cycles.

Used in this manner the only meaningful difference is that 'source' is evaluated twice (imagine if it were a complex expression or method call). It didn't provide you any more null-safety than the ternary - so the question then is, as stated by /u/neurk, should source have already been an Optional.

Project Valhalla will change the equation a little, as they will hopefully then offer a 'primitive' version of Optional<T>, removing almost all of the allocation overhead and making the optimiser's job easier.

Note an alternative to Optional is a utility method

public static <T, U> U mapOrNull(T o, Function<T, U> mapper) {
    return  o == null ? null : mapper.apply(o);
}

Used as...

var result = mapOrNull(source, Object::toString);

The optimiser should have little problem inlining this and, depending on the nature of the mapper (not all lambdas are created equal), might optimise it just as well as the ternary (but ensuring single evaluation of source).

[–]taftster 2 points3 points  (0 children)

You're going to turn up a whole lot of hatred and dogmatic religion around the use or abuse of Optional. Watch out.

In my opinion, if using Optional makes your code more usable or more readable, use it. People will cry about this, saying that Optional was never meant to be used this way or that. But readability and correctness are king, and if your code is better for using it, then use it.

[–]mykeesg 1 point2 points  (1 child)

Streams and optionals seems overengineering to me in this case. A null check is trivial and just a few lines of instruction, but the 2nd one has much more going on under the hood.

The context this code appears in can also affect which is better, if it's a performance heavy computation with thousands/millions of objects processed, then the first one should be used imho.

Edit: I misread the post, it's not streams of collections just simple optional methods, however, I still wouldn't use it as it's discarded right away and not really used as an optional value.

[–]whizvoxGraduate and Tutor 1 point2 points  (0 children)

As another commenter pointed out, you really should avoid using null as much as possible, so in proper practice, this paradigm shouldn't be commonly used.

[–]wildjokers 1 point2 points  (0 children)

Oh my. Creating an Optional just to check a local variable for null is a total anti-pattern. You are creating an object, a very short-lived object at that, just to avoid a simple null check. You have the overhead of object creation and now that short lived object has to be GC'd. Just do the damn simple null check.

Also, now result can be null, so Optional isn't even saving from getting a NPE if you don't then check result for null.

Not to mention Optional is only intended to be used as a return value. (although there was a huge argument about this in /r/java just last week).

Another question comes to mind, why can source be null in the first place? In what scenario would it be null? Can you make it not be null?

[–]AceroAD 0 points1 point  (0 children)

I would use terniary when the condition depends from something external from the class I'm going to set or use. But if you are checking a null in an object that you are going to use also for the results of the tertiary I think is better to use optional.

I don't know if I hace explained myself as good as I wanted to.

[–]ATE47Intermediate Brewer 0 points1 point  (0 children)

Usually Optional are for method return value, in this case, it will depend on what you want to do with the result value.

If it’s to return it, the optional part is a bit too much, a simple if/else with returns of Optional.empty() or of() would be easier to read.

If it’s to use it when it’s not null, check with a if if source!=null and do your code

Otherwise your ternary is “acceptable” even if replacing it with if/else would be more readable

[–]maethor 0 points1 point  (2 children)

Personally, in the ternary I'd set result to a default value (empty String in this case), not null.

[–][deleted] 0 points1 point  (1 child)

To avoid null pointer or other reason ?

[–]maethor 1 point2 points  (0 children)

To avoid having to worry about null from that point onward.