On Sat, 3 Sep 2022 03:26:05 GMT, ScientificWare <d...@openjdk.org> wrote:
>> I think Phil is talking about just `return`ing the result, which doesn't
>> change behavior. `""` can still `return Color.BLACK;` with the others
>> creating a new object, ditching the oddly-placed temporary `Color` object.
>
> @SWinxy,
> - if you're suggesting this
> ```
> } else {
> return colorNamed.get(strlc);
> ```
> We change the behavior (your comment) and lose the last test (color hex
> coded but without # prefix).
> - or if you have in mind
>
> } else {
> return new Color(colorNamed.get(strlc).color.getRGB(), true);
>
> We preserve the behavior but still lose the last test (color hex coded but
> without # prefix).
> - The way I interpreted @prrace comment :
> ```
> } else {
> Color color = colorNamed.get(strlc);
> if (color != null) {
> return color;
> ```
> We change the behavior :
> - In the code before this PR. The results were : null, Color.Black if "",
> and new Color Object for all other colors (the 10 named colors and all hex
> coded colors).
> - In the first implementation achieved in this PR, like the code above,
> results changed to : null, Color.Black if "", the same Object for each (149)
> named-color or hex coded color.
> - After your comment, results became : null, Color.Black if "", and new
> Color Object for all other Color (149 named or hex coded).
Mmm I completely forgot that we're using the map here, and for that we'll need
to create a `new Color` from it. My b. There could be a check to
`colorNames.has(str)` and return the `new Color(colorNames.get(str))` since
there will never be a `null` returned for a valid key.
-------------
PR: https://git.openjdk.org/jdk/pull/9825
