Once again, you as a developer, get a task to simplify the below code:
Set<Client> hasGoodCreditHistory (Set<Client> clients) {
Set<Client> clientsWithGoodCreditHistory = new HashSet();
for (Client client: clients) {
Payments payments = client.getCredit().getPayments();
if (payments.notOverdue())
clientsWithGoodCreditHistory.put(client);
}
return clientsWithGoodCreditHistory;
}
You can simplify a little with streams:
Set<Client> hasGoodCreditHistory (Set<Client> clients) {
return clients.stream.filter (client -> {
Payments payments = client.getCredit().getPayments();
return payments.notOverdue();
}).collect (Collectors::toSet);
}
but this is not what we want to achieve, because the logic/condition is sunk in collection processing and testing is not easy
//test
Client goodClient = new Client (isGood);
Client badClient = new Client (isBad);
Set<Client> clients = Set.of (goodClient, badClient)
Set<Client> goodCreditHistoryClients = hasGoodCreditHistory (clients);
Assert.true(goodCreditHistoryClients.contains(goodClient);
Assert.false(goodCreditHistoryClients.contains(badClient);
and as you see, leads to testing many cases (good client, bad client) in one test case, which is a bad practice.
What to make it better?
Extract the logic to separate function from collection processing :
Set<Client> hasGoodCreditHistory (Set<Client> clients) {
return clients.stream.filter (::hasGoodCreditHistory)
.collect (Collectors::toSet);
}
//or traditional foreach loop
Set<Client> hasGoodCreditHistory (Set<Client> clients) {
for (Client client: clients) {
...
}
bool hasGoodCreditHistory (Client client) {
Payments payments = client.getCredit().getPayments();
return payments.notOverdue();
}
Benefits
- simple testing or even trivial testing
Now, we need only to test the function with logic:
//test good client
Assert.true(hasGoodCreditHistory (new Client (isGood))
//test bad client
Assert.false(hasGoodCreditHistory (new Client (isBad))
There is no need to test collection processing, in this simple case.
- enforce good testing practices
When testing the list of objects, it is easy to test many cases in one test. This is not desired. One test = one test case. Simple logic enclosed function enforces this rule.
- clarity
We have enclosed a logic in a separate function, so it is clearly visible.
thx