ptuomola commented on pull request #1207:
URL: https://github.com/apache/fineract/pull/1207#issuecomment-673975607
@thesmallstar Finally had a chance to look at this and to test it out
locally. The functionality itself works as expected - great work!
The only concern I have is: I really don't like the fact that we need to
maintain two lists of ClientStatus values - one in the enum itself, another one
in the validator. It's really easy for these to get out of synch: in fact they
have already done so, as there's a spelling mistake in the validator
("transfer_in_progreess") which doesn't match the validator.
Given this, maybe the right thing to do after all is to use the ClientStatus
enum itself to validate? I.e. call the fromString method to validate, and throw
an error if you get ClientStatus.INVALID? I know that doesn't allow you to
search for values that actually have the status INVALID, but I don't think
that's a valid status in the first place - so we should not have any such
records anyway.
Does that make sense? That would allow us to remove the arrayList of valid
values, but otherwise keep the functionality intact. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
