sialcasa / mvvmFX

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

CompositeValidator valid percentaje #481

Closed jpitchardu closed 7 years ago

jpitchardu commented 7 years ago

Hi, guys

I've come with the need of displaying the percentage of completion of a form and we decided to display the percentage of valid inputs in the form, so I looked on CompositeValidator to see if there was a way to get the validators or at least the number of validators in the composite and there is not.

Although I know this is just a really really really backlog feature, it would be nice to have it.

Thanks guys

manuel-mauky commented 7 years ago

Hi Jonathan, we could add a getValidators method that returns an unmodifiable list of the validators of the CompositeValidator. Would this solve your use case?

jpitchardu commented 7 years ago

Yes it would, it'd a lot easier than saving an List

manuel-mauky commented 7 years ago

Short update: @gbalderas is working on this. It's a simple change but I also like to have some tests that show the usage of this feature. He has prepared a test that shows your use case with the percentage of successful validators. A PullRequest will be ready within a short time.

jpitchardu commented 7 years ago

Thanks a lot guys

gbalderas commented 7 years ago

Hello, while doing the test, I noticed that if the same error message is used by different Validators, all of them will be removed from the HashMap of messages in the CompositeValidationStatus (line 136). This happens when more than one Validator are valid.

When this happens my test fails since my Binding cannot be updated because it is bound to the messages ObservableList. Also the list of messages in the CompositeValidator will be reduced to 1, making it impossible to know how many validators are valid based on the message count. But if I do a for loop for each element in the validators list, I can count how many of them are valid.

Is this intentional? Should every validator message be removed if there are used more than once? Or should every Validator have a different/unique message?

If it is intentional, maybe the CompositeValidator can give a number or an observable value of how many validators are valid?

manuel-mauky commented 7 years ago

Hi @gbalderas, thanks for your work. The problem that you describe sounds like you are using the same instance of Error for multiple validators. We introduces the HashMap that you've mentioned as a fix for other problems. But maybe we have overlooked some edge cases and maybe we should document this behaviour better.

However, I don't think this is connected to the actual issue we are trying to solve here. Can you please create a new issue for this particular problem and create the PullRequest for this issue here anyway?

manuel-mauky commented 7 years ago

@pichardoJ can you please take a look at PR #486. Does this solve your use case?

jpitchardu commented 7 years ago

@lestard yes it does, thank you and @gbalderas for the quick response