sialcasa / mvvmFX

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

Potential bug in ValidationVisualizerBase #539

Closed ntherrien closed 6 years ago

ntherrien commented 6 years ago

Consider the following code:

public abstract class ValidationVisualizerBase implements ValidationVisualizer {
    public ValidationVisualizerBase() {
    }

    public void initVisualization(ValidationStatus result, Control control, boolean required) {
        if (required) {
            this.applyRequiredVisualization(control, required);
        }

        this.applyVisualization(control, result.getHighestMessage(), required);
        result.getMessages().addListener((c) -> {
            while(c.next()) {
                Platform.runLater(() -> {
                    **this.applyVisualization(control, result.getHighestMessage(), required);**
                });
            }

        });
    }

    abstract void applyRequiredVisualization(Control var1, boolean var2);

    abstract void applyVisualization(Control var1, Optional<ValidationMessage> var2, boolean var3);
}

Is it possible that: this.applyVisualization(control, result.getHighestMessage(), required); should be showing additional messages rather than the highest? Why are we looping in getMessages() if not for doing something with c? this.applyVisualization(control, c, required);

ntherrien commented 6 years ago

@lestard if you want i can submit a fix for this bug. Assign this issue to me and I will file a pull request.