Why should you avoid conditional logic in unit tests and how? [closed]

Question

Imagine having the following classes:

public class Product {
   private String name;
   private double price;

   // Constructors, getters and setters
}

public class Products {
   private List<Product> products;

   // CRUD methods

   public double getTotalPrice() {
      // calculates the price of all products
   }
}

I have read that conditional logic(if's and loops) should be avoided in unit tests, but didn't understand exactly why and more importantly how. How can I efficiently test the scenario where I add some products in Products with different prices and then verify the result of getTotalPrice() without using loops?

Answer 1

The fear with conditionals and loops is two-fold:

1. You may have a bug in your TEST code so the test is not run properly or worse an assertion inside the conditional block is not asserted.
2. It is harder to read.

Other people have answered to just hard code the list via copy and paste. I don't like that because not only does it clutter the test but it makes it harder to refactor later.

If the code was like in Invisible Arrow's answer:

@Test
public void testsTotalPriceAsSumOfProductPrices() {
    Products products = new Products(); // Or any appropriate constructor
    products.add(new Product("first", 10)); // Assuming such a constructor exists
    products.add(new Product("second", 20));
    products.add(new Product("second", 30));
    assertEquals("Sum must be eq to 60", 60, products.getTotalPrice());
}

And the constructor for Product changed, you would have to change all the test code in lots of different places.

I prefer to make helper methods that make the test code more intent revealing and keep the number of places to change during refactoring to a minimum (hopefully just one).

Isn't this easier to read:

 @Test
public void testsTotalPriceAsSumOfProductPrices() {
    Products products = new Products(); 

    addProductsWithPrices(products, 10,20,30);

    assertEquals(60, products.getTotalPrice());

}

private static void addProductsWithPrices(Products products, Double...prices){
  for(Double price : prices){
     //could keep static counter or something
     //to make names unique
     products.add(new Product("name", price));
  }
}

Yes this does use a for loop. But if you are worried about it having bugs in it or if the helper methods are more complicated, you can write additional tests for them too! Eventually you may want to factor these helper methods out to their own classes so they can be reused in other tests.

In addition, you can see that the test method hides the other fields that are required to make products (name) that are irrelevant to the test. Our test only cares about prices so our helper just makes up names and the person reading the test doesn't get confused by extra parameters. It's clear that this test makes sure 10+20+30 ==60.

Finally, if we ever change our price from a double to somekind of Currency object we only have to make the change in once place and our test code is just as readable.

 private static void addProductsWithPrices(Products products, Double...prices){
  for(Double price : prices){
     //could keep static counter or something
     //to make names unique
     products.add(new Product("name", Currency.valueOf(price)));
  }
}

Answer 2

Reason for avoiding conditionals and loops in unit tests is because they become hard to read and maintain. Each conditional would most likely represent a test scenario of its own, and should be in a separate unit test. Each test will then act as a document or spec for the functionality. A person looking at the tests will know exactly what is expected/not expected of the functionality.

For your particular case above, you could go with a simplistic approach and test the summing function by adding a few products to the list.

public class ProductsTest {
    @Test
    public void testsTotalPriceAsSumOfProductPrices() {
        Products products = new Products(); // Or any appropriate constructor
        products.add(new Product("first", 10)); // Assuming such a constructor exists
        products.add(new Product("second", 20));
        products.add(new Product("second", 30));
        assertEquals("Sum must be eq to 60", 60, products.getTotalPrice());
    }
}

In my opinion, using a loop above to either create/insert items is not necessary and adds complexity to the test.

You could of course have more tests that test boundary conditions such as sum overflows, exclusion of negative numbers etc.