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