RFR: JDK-8292276 : Missing color names in CSS. [v3]

[ScientificWare]

On Tue, 16 Aug 2022 00:26:47 GMT, SWinxy <duke at openjdk.org> wrote:

>> ScientificWare has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Adds to CSS.java the missing color names.
>>   
>>   Adds to CSS.java, the missing color names defined by :
>>   CSS Color Module Level 4
>>   W3C Candidate Recommendation Snapshot, 5 July 2022
>>   [7.1 Named Colors](https://www.w3.org/TR/css-color-4/#named-color)
>>   - Updates, Copyright year.
>>   - Adds relative imports.
>>   - Replaces, if ... then ... else statements with a Map called "colorNamed".
>>     Code is more readable and easy to maintain.
>>     After tests, TreeMap seems slower than Map. Results are available at scientificware#12 (comment).
>>   
>>   Warning : The Previous JDK CSS Orange Color is different from CSS Color Recommendation.
>
> Hey there. The current implementation creates a new `Color` object for each invocation of `stringToColor` (with the exception of `""` because we want to keep developers on their toes). Using a `Map` will *not* create new `Color` objects for each invocation, which may explain why your results show `Map` as the most performant. This is *technically* a change in behavior, and *technically* not wanted.
> 
> To get around this, you can do `new Color(c.getRed(), c.getGreen(), c.getBlue())` from the map, or--what I would prefer--to use an enhanced switch statement to create the colors.
> 
> One thing I'd request is to have `stringToColor` return in the branches, rather than setting the placeholder `Color color;` object. Things like this irk me. As in (using the map):
> 
> static Color stringToColor(String str) {
>         if (str == null) {
>             return null;
>         } else if (str.length() == 0) {
>             return Color.black;
>         } else if (str.startsWith("rgb(")) {
>             return parseRGB(str);
>         } else if (str.startsWith("rgba(")) {
>             return parseRGBA(str);
>         } else if (str.charAt(0) == '#') {
>             return hexToColor(str);
>         } else if (colorNamed.containsKey(str.toLowerCase(Locale.ROOT))) {
>             return colorNamed.get(str.toLowerCase(Locale.ROOT));
>         } else {
>             return hexToColor(str);
>         }
>     }
> 
> 
> In general, good PR. I'd be interested to know the perf results when the behavior is unchanged, and if the enhanced switch would win out.

@SWinxy Thanks for comments. After reading them, I realize the Big Mistake !
I Shouldn't create a new object but rather return an existing one.

That was in my intention. All the code is already available [here](https://github.com/scientificware/jdk/issues/12), I wrote it because I expected to move this logic to Color.java.

All color declarations are ready.

The Map will be something like : 


static TreeMap<String, Color> colorNamed = new TreeMap<String, Color>(
        Map.ofEntries(
            Map.entry("aliceblue", ALICE_BLUE),
            ...
            Map.entry("yellowgreen", YELLOW_GREEN)
        );            

Do you validate this approach ?

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

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