Extract a logic from collections processing


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

,

Leave a Reply

Your email address will not be published. Required fields are marked *