On Sat, 3 Sep 2022 23:40:02 GMT, SWinxy <[email protected]> wrote: >> @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.
The old code didn't have any cache, it simply returned a color. Now that the colors are in a map, return the `Color` instance from the map if there's one. ------------- PR: https://git.openjdk.org/jdk/pull/9825
