RFR: JDK-8293776 : Adds CSS 4 and 8 digits hex coded Color [v6]
Alexey Ivanov
aivanov at openjdk.org
Mon Sep 26 11:07:24 UTC 2022
On Fri, 23 Sep 2022 22:25:09 GMT, ScientificWare <duke at openjdk.org> wrote:
> Could we reach a conciliation with this version. ? All cases are treated in 2 or 3 tests.
Your new versions introduce one more condition to each case and make the code more complicated and less clear. I am for concise, clear and readable code. I am against premature optimisation, and it hasn't been proved yet that there's a performance issue.
Combining the cases of `n == 3` and `n == 4` avoids code duplication by introducing one more comparison to both cases. If this becomes a critical path, JIT could optimise the code.
As such, I am still for combining these two cases. If you insist, I'm okay with your original code with code duplication. Yet I'd like to eliminate the duplicate code.
> Some articles suggest to avoid using exceptions as flow controls. I never tested this point. That's why I introduced Patterns in this method.
It's true, creating an exception object is a somewhat expensive operation. Yet you say, “_most_ of color notations are correct”, therefore an incorrect color which cannot be parsed is rare, so it's _an exceptional situation_, it is okay to use exceptions in this case. You also say, “Pattern usage has a significant time cost,” at the same time, you introduce a small time penalty for each and every correct color.
I think the hex pattern is redundant.
> I also wish to test the version with bit rotation too :
>
> ```java
> return new Color(Integer.rotateRight(Integer.parseUnsignedInt(digits, 16),8),
> true);
> ```
It could be not as clear, yet it works.
May I suggest wrapping the code for `Color` and `rotateRight` arguments?
return new Color(
Integer.rotateRight(Integer.parseUnsignedInt(digits, 16),
8),
true);
It fits better into 80-column limit. Alternatively, wrap the second parameter of `rotateRight` only:
return new Color(Integer.rotateRight(Integer.parseUnsignedInt(digits, 16),
8),
true);
This way it's easier to see how many bits are rotated.
-------------
PR: https://git.openjdk.org/jdk/pull/10317
More information about the client-libs-dev
mailing list