Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of Optionals in Java 8 #94

Open
CLOVIS-AI opened this issue Apr 12, 2018 · 13 comments
Open

Use of Optionals in Java 8 #94

CLOVIS-AI opened this issue Apr 12, 2018 · 13 comments

Comments

@CLOVIS-AI
Copy link

Since Java 8, Optionals were added to replace null when a method can not return a result, as seen in JsonObject with the methods get, getInt ...

Although the current use of default values is convenient, I think it would be good to allow the use of Optionals, as they allow more control.

I am ready to make the changes myself and submit a pull request, but I'm not sure if it's better, regarding this project's policy, to do it:

  1. replace the current methods' return type by Optionals (can cause problems with dependancies)
  2. add new methods (like getOptional, getIntOptional), which can make the code a bit "messy" (and maybe deprecate the old ones)

Do you think it's worth doing it, and if it is, which would be the best way (I mean, the most user-friendly way) of implementing it?

@bernardosulzbach
Copy link
Contributor

I think this is a welcome change. First of all, and quite evidently, changing existing signatures will cause compatibility problems. So it should be done through new methods.

However, I don't know if this is justified. The problem that having a default value returned does not tell you whether there was a value or not is solved by simply checking for member existence. Additionally, I'd rather read a check for presence with has... rather than a test to see if an optional is empty or not.

@CLOVIS-AI
Copy link
Author

What I meant, is the others methods than plain simple Optional#get, like:

  • Optional#ifPresent(lambda-expression)
  • Optional#flatMap(lambda-expression)
  • Optional#orElse(value) (the equivalent of the current system with default values)
  • Optional#orElseGet(lambda-expression)
  • Optional#orElseThrow(lambda-expression)
  • ...

All of which are very convenient. Optional official documentation

@sroughley
Copy link

It would be great to have these in the API, but retain the original methods too, which has advantages of not breaking existing API and also avoiding object creation when a simple null check will suffice.

@CLOVIS-AI
Copy link
Author

That would mean adding methods like getOptional, getOptionalInt etc, right?

@sroughley
Copy link

Yes, that was how I understood the suggestion - I would assume in each case the actual method would be something very simple like

Optional<JsonValue> get(String name) {
    return Optional.ofNullable(get(name));
}

@CLOVIS-AI
Copy link
Author

Mostly, with some differences for primitives because they cannot be null.

@sroughley
Copy link

Interesting point. Most of those methods have a default value as a parameter, so would you just put that into e.g. an OptionalInt instead, in which case you would have something like:

OptionalInt getOptionalInt(String name, int default){
    return OptionalInt.of(getInt(name, default));
}

(which looks a bit pointless?!), or would you provide a new method something like:

OptionalInt getOptionalInt(String name){
    JsonValue value = get(name);
    return value != null ? OptionalInt.of(value.asInt()) : OptionalInt.empty();
}

(which looks more useful!) Or, I suppose you could do both..?

@CLOVIS-AI
Copy link
Author

I was thinking about the second version.

@CLOVIS-AI
Copy link
Author

CLOVIS-AI commented Apr 13, 2018

Also, because the methods getInt always have 2 parameters (name & default value), we could simply do something like:

int getInt(String name, int default);
OptionalInt getInt(String name);

This way, both ways can cohabit nicely in an intuitive manner?

@CLOVIS-AI
Copy link
Author

Well, based on this discussion, I'll open a pull request this week with it

@CLOVIS-AI
Copy link
Author

I am currently working on it, and I stumbled upon a problem: something changed between Java 7 and Java 8 in the language specifications related to type inference.

For example, if I write:

Optional<Float> getFloat(String name){
  return ... : Optional.empty()
}

the compiler recognizes Optional.empty() as returning Optional<? extends Object>, but since Java 8 it should use type inference to guess that the return type is Optional<Float>. Related question on StackOverflow

I have tried this with the exact same code in a project with only the source code and not the project configuration and it works fine, so I'm sure it comes from the project configuration. NetBeans has hinted that it's related to -source 1.5 that should be -source 1.8, but that is still pretty cryptic to me. Since it's related to project configuration, I thought it would be better to ask the code owners what they think on this issue.

@CLOVIS-AI CLOVIS-AI mentioned this issue Apr 14, 2018
@sroughley
Copy link

I think you should be able to use

return ... : Optional.<Float>empty();

based on pg 138 of Joshua Bloch's 'Efficient Java'

@CLOVIS-AI
Copy link
Author

It works, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants