[jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v3]

[Kevin Rushforth]

On Tue, 27 Jul 2021 12:44:58 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> This PR corrects/adds missing documentation for classes in javafx.css package.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8250590 - add missing @since tag

I did a pretty complete review of the docs, and left what are mostly minor (and in some cases picky) comments.

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 110:

> 108: 
> 109: /**
> 110:  * A parser for CSS document string.

I suggest either `document strings.` or `a document string`.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 36:

> 34: 
> 35: /**
> 36:  * This class serves as a container of CSS property and its value.

`a CSS property`

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 91:

> 89:     /**
> 90:      * Gets the importance of this {@code Declaration}.
> 91:      * @return important

Maybe `@return the important flag` or `@return whether this {@code Declaration} is important`?

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 113:

> 111:      * the {@code Declaration} belongs. Only the property, value and importance are
> 112:      * considered.
> 113:      * </p>

No need for a `</p>`. Also there is a missing `@param obj` tag.

modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 114:

> 112:      * considered.
> 113:      * </p>
> 114:      * @return true if this object is the same as the obj argument; false otherwise.

I recommend code style for `true`, `obj`, and `false`.

modules/javafx.graphics/src/main/java/javafx/css/PseudoClass.java line 87:

> 85:      * <p>
> 86:      * Note: There is only one {@code PseudoClass} instance for a given pseudo class name.
> 87:      * </p>

No need for `</p>`.

modules/javafx.graphics/src/main/java/javafx/css/PseudoClass.java line 99:

> 97:     /**
> 98:      * Gets the name of the {@code PseudoClass}.
> 99:      * @return the name of the {@code PseudoClass}.

You can remove the `.` here.

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 101:

> 99: 
> 100:     /**
> 101:      * Same as the matches method expect return true/false rather than a match.

Perhaps add an initial sentence like `Gets whether this {@code Selector} applies to the given {@code Styleable}.`? This (now second) sentence could use a little rewording to make it a complete sentence and `matches` could maybe be a link.

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 103:

> 101:      * Same as the matches method expect return true/false rather than a match.
> 102:      * @param styleable styleable to match.
> 103:      * @return {@code true} if this {@code Selector} applies to the given {@code Styleable}.

No need for periods for these two tags.

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 108:

> 106: 
> 107:     /**
> 108:      * Same as applies, but will return pseudoclass state that it finds along the way.

Same comment as above.

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 113:

> 111:      * @param depth depth of the {@code Node} heirarchy to look for
> 112:      * @return {@code true} if this {@code Selector} and a set of {@code PseudoClass}
> 113:      * applies to the given {@code Styleable}.

No need for period here.

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 133:

> 131:      * Writes {@code Selector} data in binary form to given {@code DataOutputStream}.
> 132:      * @param os {@code DataOutputStream} to write {@code Selector} data to
> 133:      * @param stringStore unsused

Typo: `unsused` --> `unused`

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 163:

> 161:     /**
> 162:      * Creates a {@code Selector} object.
> 163:      * @param cssSelector css selector string

`CSS` should be upper case (not the parameter name, of course)

modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 164:

> 162:      * Creates a {@code Selector} object.
> 163:      * @param cssSelector css selector string
> 164:      * @return a Selector

`selector` or `{@code Selector}`

modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 74:

> 72:     /**
> 73:      * Gets Immutable List of style-classes of the selector.
> 74:      * @return Immutable List<String> of style-classes of the selector

Maybe better in this case to just say `list` (lower case) for both the description and `@return`? Otherwise, use code style. Also, "immutable" should not be capitalized.

modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 90:

> 88:     /**
> 89:      * Gets the {@code Set} of {@code StyleClass} of the selector.
> 90:      * @return set of style class

style classes (plural)

modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 151:

> 149:     // Used in Match. If nodeOrientation is ltr or rtl,
> 150:     // then count it as a pseudoclass
> 151:     /** Gets {@code NodeOrientation} of this Selector.

Minor: move the text to the next line so the `/**` is on a line by itself.

modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 152:

> 150:     // then count it as a pseudoclass
> 151:     /** Gets {@code NodeOrientation} of this Selector.
> 152:      * @return nodeOrientation

either `node orientation` or `{@code NodeOrientation}`

modules/javafx.graphics/src/main/java/javafx/css/StyleConverter.java line 567:

> 565: 
> 566:         /**
> 567:          * Writes the StringStore strings to a given {@code DataOutputStream}.

`{@code StringStore}`

modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java line 41:

> 39: 
> 40: /**
> 41:  * Converter to convert representation of an {@code Effect} to an {@code Effect}.

`a representation` or maybe `a string representation`?

modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java line 82:

> 80: 
> 81:     /**
> 82:      * Converter to convert DropShadow {@code Effect}.

Maybe `Converter to convert a {@code DropShadow}.` or  `Converter to convert a {@code DropShadow} effect.`

modules/javafx.graphics/src/main/java/javafx/css/converter/EnumConverter.java line 127:

> 125:      */
> 126:     // package for unit testing
> 127:     static public StyleConverter<?,?> getInstance(final String ename) {

Independent of the doc change, the inline comment is wrong: This is a public, not a package-scope method. Maybe remove the comment?

modules/javafx.graphics/src/main/java/javafx/css/converter/StringConverter.java line 47:

> 45:     /**
> 46:      * Get the {@code StringConverter} instance.
> 47:      * @return the {@code StringConverter} instance.

You can remove the period.

modules/javafx.graphics/src/main/java/javafx/css/converter/StringConverter.java line 72:

> 70: 
> 71:     /**
> 72:      * Converter to convert a sequence of {@code String}s to a String[].

I recommend code style for the `String[]` or else say `an array of {@code String}s`.

modules/javafx.graphics/src/main/java/javafx/css/converter/StringConverter.java line 79:

> 77:         /**
> 78:          * Get the {@code SequenceConverter} instance.
> 79:          * @return the {@code SequenceConverter} instance.

Extra period.

modules/javafx.graphics/src/main/java/javafx/css/converter/URLConverter.java line 254:

> 252: 
> 253:     /**
> 254:      * Converter to convert a sequence of URLs to a String[].

Same comment as above about using code style for `String[]`

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

PR: https://git.openjdk.java.net/jfx/pull/589