-
-
Notifications
You must be signed in to change notification settings - Fork 48
Miscellaneous character.jsp cleanups #1063
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
Conversation
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.
looks good, thanks!
UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeUtilities.java
Outdated
Show resolved
Hide resolved
UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeUtilities.java
Outdated
Show resolved
Hide resolved
UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeUtilities.java
Outdated
Show resolved
Hide resolved
UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeUtilities.java
Outdated
Show resolved
Hide resolved
UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeUtilities.java
Outdated
Show resolved
Hide resolved
: Collections.singleton(getValue(codepoint)); | ||
String value = getValue(codepoint); | ||
return isMultivalued && value != null | ||
? delimiterSplitter.split(value) |
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.
optional: does this return a Collection? if so, upgrade the return type?
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.
It returns an Iterable. This is a Google thing. Its documentation says Java 8 users may prefer {@link #splitToStream} instead.
. Do we prefer that?
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 think I am at least as confused by the Iterable/Collection/Stream situation as you are by C++ iterators…
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 think I am at least as confused by the Iterable/Collection/Stream situation as you are by C++ iterators…
I bet I am still more confused ;-)
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.
It returns an Iterable. This is a Google thing. Its documentation says
Java 8 users may prefer {@link #splitToStream} instead.
. Do we prefer that?
Maybe. For the singleton case, we could then presumably use Stream.of(value)
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.
we could then call this valueStream()
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.
Streams are the preferred form where you normally just want to deal with the contents one at a time. You can always create a iterator from a stream.
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’ll do that in a later PR, since it is going to affect other callers.
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.
teaching each other programming ;-)
<wbr>
between values of multivalued properties so exemplar doesn’t make https://util.unicode.org/UnicodeJsps/character.jsp?a=a stupidly wide.