ptuomola commented on pull request #1207:
URL: https://github.com/apache/fineract/pull/1207#issuecomment-668755830


   > @ptuomola I am collectively answering to your doubts here:
   > 
   > 1. I  am actually adding clients, so there would be some clients in the 
database for sure. (Check the first for loop).
   > 2. I am adding 15 clients and each time I am assigning him pending or 
active status randomly, that means
   > 
   > The probability that there are no active/pending clients = 1/pow(2,15) = 
0.00003051757, I hope you will agree we can ignore this and consider that there 
is at least one active/pending client.
   
   No that wasn't my concern - I know you create active and pending clients. My 
concern was: what if the query selection doesn't work as expected, and instead 
returns an empty list without matching any clients? I don't see the test 
checking for the case where there is no clients returned - it just checks that 
whatever clients are returned are in the correct status. 
   
   So something like assertNotEquals(list.size(), 0) for both cases...
   
   > 1. The client status never actually becomes invalid, it just means that 
there is no such status. In that case, we as you said we can either give an 
exception or show all entries.
   >    @ptuomola as you said, should we instead throw an exception? still 
confused or we should instead show all the clients if it does not map to any of 
them.
   
   My personal view would be that we should throw an exception if the caller 
sends a status that is not a valid value for ClientStatus. Otherwise the caller 
thinks that there are no clients matching this status and never finds out that 
they actually sent the wrong value. Empty list should mean that there are no 
matches. 
   


----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to