-
Notifications
You must be signed in to change notification settings - Fork 164
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
GH-688: Added getLabel OptionalMethod for Optional #2279
GH-688: Added getLabel OptionalMethod for Optional #2279
Conversation
Signed-off-by: SakshiSaini17092 <sakshi17092@iiitd.ac.in>
@@ -51,6 +48,11 @@ public static String getLabel(Value v, String fallback) { | |||
return v instanceof Literal ? getLabel((Literal) v, fallback) : fallback; | |||
} | |||
|
|||
public static String getLabel(Optional v, String fallback) { | |||
|
|||
return v instanceof Optional ? getLabel((Value) v.orElseGet(null), fallback) : fallback; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to check if v
is an Optional
- it always is (because that's the type of the argument) - though perhaps you could check that it is not null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I fixed that in my latest commit.
import java.util.IllformedLocaleException; | ||
import java.util.Locale; | ||
import java.util.Objects; | ||
import java.util.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is something that your editor has done automatically - could you change it? We don't use wildcards for imports.
If you are using Eclipse IDE, it's really easy to change this btw: go to 'Preferences' -> 'Java' -> 'Code Style' -> 'Organize imports', and where it says "number of imports needed for .*" set it to some very high number (99 for example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is also done.
Signed-off-by: SakshiSaini17092 <sakshi17092@iiitd.ac.in>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
If you haven't already, could you comment on #2235 and leave your names, so we can include them in the release notes? Thanks. |
Signed-off-by: SakshiSaini17092 sakshi17092@iiitd.ac.in
GitHub issue resolved: #688
Briefly describe the changes proposed in this PR:
Added getLabel function for Optional.
PR Author Checklist (see the contributor guidelines for more details):
mvn process-resources
to format from the command line)Note: we merge all feature pull requests using squash and merge. See RDF4J git merge strategy for more details.