Review patches isBMPCodePoint/2/3

[Ulf Zibis]

Am 23.03.2010 19:19, schrieb Martin Buchholz:
> Ulf,
>
> Please do not delete methods in Surrogate.java
> (because we take compatibility seriously)
>    

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint

I still think, we should stick on Surrogate#isBMP for above 
compatibility reason.
Otherwise we too should rename #neededFor etc.
Please add @author Ulf Zibis and correct copyright date.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint2

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint2/src/share/classes/java/lang/AbstractStringBuilder.java.sdiff.html
Looks good, but please add @author Ulf Zibis and correct copyright date.
http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint2/src/share/classes/java/lang/Character.java.sdiff.html
Looks good, but please add @author Ulf Zibis and correct copyright date.
http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint2/src/share/classes/java/lang/String.java.sdiff.html
Looks good, but please add @author Ulf Zibis and correct copyright date.
http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint2/src/share/classes/sun/nio/cs/Surrogate.java.sdiff.html
Looks good, but I still think we should use deprecated.
"deprecate" just means "don't use it if even possible", IMO not only for 
APIs that are actively harmful.
Imagine, users code relies on existing sun package API, because there 
was no appropriate public API in the past.
He is *used to ignore* the warning: Surrogate is Sun proprietary API and 
may be removed in a future release.
"Deprecated" will give him new attention, so it's likely, he will 
notice, that there are new API's since JDK 7.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint3

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/isBMPCodePoint3/test/java/nio/charset/coders/BashStreams.java.sdiff.html
         if (Character.isBMPCodePoint(c) && (c >= '\uFFFE'
                  || Character.isSurrogate((char))c)));
Please use 8-space-indentation for line continuation, following looks ugly:
  259                         if (Character.isHighSurrogate(c)
  260 && (cb.remaining() == 1)) {
  261                             cg.push(c);
  262                             break;
  263                         }


-Ulf

> but instead gently denigrate them,
> as I do below (added to my patch isBMPCodePoint2)
>
> diff --git a/src/share/classes/sun/nio/cs/Surrogate.java
> b/src/share/classes/sun/nio/cs/Surrogate.java
> --- a/src/share/classes/sun/nio/cs/Surrogate.java
> +++ b/src/share/classes/sun/nio/cs/Surrogate.java
> @@ -77,6 +77,7 @@
>       /**
>        * Tells whether or not the given UCS-4 character must be represented as a
>        * surrogate pair in UTF-16.
> +     * Use of {@link Character#isSupplementaryCodePoint} is generally
> preferred.
>        */
>       public static boolean neededFor(int uc) {
>           return Character.isSupplementaryCodePoint(uc);
> @@ -102,6 +103,7 @@
>
>       /**
>        * Converts the given surrogate pair into a 32-bit UCS-4 character.
> +     * Use of {@link Character#toCodePoint} is generally preferred.
>        */
>       public static int toUCS4(char c, char d) {
>           assert Character.isHighSurrogate(c)&&  Character.isLowSurrogate(d);
>
>
> Martin
>
>
>