sialcasa / mvvmFX

an Application Framework for implementing the MVVM Pattern with JavaFX
Apache License 2.0
491 stars 105 forks source link

Wrong initialize method on view model is called #529

Closed AKreisel closed 6 years ago

AKreisel commented 7 years ago

On initialiazing a view model, the FxmlViewLoader uses the method 'getInitializeMethods' In ViewLoaderReflectionUtils:

 * Returns a collection of {@link Method}s that represent initializer methods.
 * A method is an "initializer method" if it either: <br/>
 * <ol>
 *     <li>has a signature of "public void initialize()"</li>
 *     <li>is annotated with {@link Initialize}</li>
 * </ol>
 */
private static Collection<Method> getInitializeMethods(Class<?> classType) {
    final List<Method> initializeMethods = new ArrayList<>();
    Arrays.stream(classType.getMethods())
            .filter(method -> "initialize".equals(method.getName()))
            .findAny()
            .ifPresent(initializeMethods::add);

    Arrays.stream(classType.getDeclaredMethods())
            .filter(method -> method.isAnnotationPresent(Initialize.class))
            .forEach(initializeMethods::add);

    return initializeMethods;
}

While the Javadoc correctly states that all methods annotated with @Initialize and methods having the signature "public void initialize()" are initializer methods, the code actually takes every method named "initialize" without checking the full signature (meaning it doesn't have parameters). This results in an exception if the view model contains only one method called "initialize" which expects at least one parameter.

manuel-mauky commented 7 years ago

Thanks for this hint. You are right. The full signature should be checked. By looking at this code I also think that we should use a Set instead of a List so that methods aren't discovered a second time. This would happen if a method "initialize" is also annotated with "@Initialize".