RFR: JDK-8293776 : Adds CSS 4 and 8 digits hex coded Color [v6]

ScientificWare duke at openjdk.org
Fri Sep 23 22:27:16 UTC 2022


On Fri, 23 Sep 2022 16:48:40 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> With this change, the code is more concise but we double the number of tests ?
>
>> With this change, the code is more concise but we double the number of tests ?
> 
> For a rather rare case. I don't think it's a critical path which affects performance.
> 
> If you like, you can create a variable to store the result of `n == 4` to avoid testing once again. I still don't think it's worth it.

Could we reach a conciliation with this version. ? All cases are treated in 2 or 3 tests.
It incorporates all your suggestions and my single comment about the number of tests.

        if (n < 5) {
            if (n < 3) {
                return null;
            } else {
                final char r = digits.charAt(0);
                final char g = digits.charAt(1);
                final char b = digits.charAt(2);
                final char a = n == 4 ? digits.charAt(3) : 'f';
                digits = String.format("%1$s%1$s%2$s%2$s%3$s%3$s%4$s%4$s", r, g, b, a);
            }
        } else if (n < 7) {
            if (n == 5) {
                return null;
            } else {
                digits = String.format("%sff", digits);
            }
        } else if (n != 8 ) {
            return null;
        }
        try {
            int i = Integer.parseUnsignedInt(digits, 16);
            return new Color((i >> 24) & 0xFF, (i >> 16) & 0xFF, (i >> 8) & 0xFF, i & 0xFF);
        } catch (NumberFormatException nfe) {
            return null;
        }

Some articles suggest to avoid using exceptions as flow controls. I never tested this point. That's why I introduced Patterns in this method. May I run tests in this case ? With the code below :

        if (n < 5) {
            if (n < 3 || !hex.matcher(digits).matches()) {
                return null;
            } else {
                final char r = digits.charAt(0);
                final char g = digits.charAt(1);
                final char b = digits.charAt(2);
                final char a = n == 4 ? digits.charAt(3) : 'f';
                digits = String.format("%1$s%1$s%2$s%2$s%3$s%3$s%4$s%4$s", r, g, b, a);
            }
        } else if (n < 7) {
            if (n == 5 || !hex.matcher(digits).matches()) {
                return null;
            } else {
                digits = String.format("%sff", digits);
            }
        } else if (n != 8 || !hex.matcher(digits).matches()) {
            return null;
        }
        try {
            int i = Integer.parseUnsignedInt(digits, 16);
            return new Color((i >> 24) & 0xFF, (i >> 16) & 0xFF, (i >> 8) & 0xFF, i & 0xFF);
        } catch (NumberFormatException nfe) {
            return null;
        }

I also wish to test the version with bit rotation too :

        if (n < 5) {
            if (n < 3 || !hex.matcher(digits).matches()) {
                return null;
            } else {
                final char r = digits.charAt(0);
                final char g = digits.charAt(1);
                final char b = digits.charAt(2);
                final char a = n == 4 ? digits.charAt(3) : 'f';
                digits = String.format("%1$s%1$s%2$s%2$s%3$s%3$s%4$s%4$s", r, g, b, a);
            }
        } else if (n < 7) {
            if (n == 5 || !hex.matcher(digits).matches()) {
                return null;
            } else {
                digits = String.format("%sff", digits);
            }
        } else if (n != 8 || !hex.matcher(digits).matches()) {
            return null;
        }
        try {
            return new Color(Integer.rotateRight(Integer.parseUnsignedInt(digits, 16),8),
                             true);
        } catch (NumberFormatException nfe) {
            return null;
        }

-------------

PR: https://git.openjdk.org/jdk/pull/10317



More information about the client-libs-dev mailing list