On Fri, 23 Sep 2022 22:25:09 GMT, ScientificWare <d...@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
