RFR: 8313651: Add 'final' keyword to public property methods in controls [v2]
Andy Goryachev
angorya at openjdk.org
Mon Aug 21 15:25:35 UTC 2023
On Sun, 20 Aug 2023 21:17:18 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>>
>> review comments
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlPropertiesTest.java line 232:
>
>> 230: private String err(Method method, String message) {
>> 231: return method + " " + message;
>> 232: }
>
> (This comment didn't appear in the review for some reason.)
>
> I think that the code could be a bit more clean. I took a stab at it:
>
>
> Set<Class<?>> CLASSES = Set.of(/*...*/);
>
> private void check(Class<?> cls) {
> Map<String, Method> methods = Stream.of(cls.getMethods()).collect(Collectors.toMap(Method::getName, m -> m)); // the loop is also fine
>
> for (var method : methods.values()) {
> String name = method.getName();
> if (name.endsWith("Property")) {
> checkModifiers(method);
>
> String propName = name.substring(0, name.length() - "Property".length());
> check(methods, propName, "get", 0);
> check(methods, propName, "set", 1);
> check(methods, propName, "is", 0);
> }
> }
> }
>
> private void check(Map<String, Method> methods, String name, String prefix, int numArgs) {
> String methodName = new StringJoiner("", prefix, name.substring(0, 1).toUpperCase() + name.substring(1)).toString();
>
> Method m = methods.get(methodName);
> if (m != null && m.getParameterCount() == numArgs) {
> checkModifiers(m);
> }
> }
>
> private void checkModifiers(Method m) {
> int mod = m.getModifiers();
> if (Modifier.isPublic(mod) && !Modifier.isFinal(mod)) {
> if (FAIL_FAST) {
> throw new AssertionError(errorMessage(m));
> } else {
> System.err.println(errorMessage(m));
> }
> }
> }
>
> private String errorMessage(Method method) {
> return method + "is not final";
> }
Thank you for suggestions!
Frankly, I don't see much value in rewriting of this method. A more interesting problem is how to enumerate all descendants of the Control class using the package name, in such a way that will reliably work in an IDE, from a JAR, or in any test framework (jtreg or otherwise).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1213#discussion_r1300278200
More information about the openjfx-dev
mailing list