IEnumerable considered harmful (Part 2)
This is part 2 of the IEnumerable considered harmful series. You can read the first part here.
Multiple Evaluation
Another common mistake when using IEnumerable is treating such methods as views over existing collections. Such methods often generate collection items. This means that the collection is created on demand and recreated on each call.
Let’s take a look at the example of ResponseCodeCheckService. This service has method CheckErrorsInResponseCodes which:
- Takes a list of response codes to check
- Compares this list against a list of known error codes
- Returns true if none of the response codes match any error code
At first glance there’s nothing wrong with our code. However, as diligent developers do, we wrote a test.
We’ve set the Timeout for this test at 1 second. We expect this test to pass as the SomeLongRunningOperation takes about half a second to complete. But the test fails. It takes about 1.5 seconds to complete!
Checking the service code we can quickly deduce GetErrorCodes is being called for each response code to check. So it makes sense we’ve exceeded the predicted performance parameters. We confirm this by adding another test.
The test fails. To make this test pass, we need to make sure GetErrorCodes is called only once by fixing our service method.
However, the result is not what we expected. GetErrorsCalledOnce succeeds but our original test still times-out!
Although GetErrorCodes is called only once, SomeLongRunningOperation is called multiple times. We confirm this by writing another test.
This test fails. To understand the issue, we must look at the return value of GetErrorCodes method.
It returns an IEnumerable<T>. The return value of this method is not some “collection of items”, but simply an object that implements a method GetEnumerator, which returns an IEnumerator<T>.
This IEnumerator is an object, that implements various methods, such as Current and MoveNext, which return values of specified generic type. The variable errorCodes in our code does not point to a collection of error codes. It points to an object capable of returning an IEnumerator, which is capable of returning such codes.
When our code requests the values of such “collection” in the Linq method, we’re essentially instancing IEnumerator(s), which then go about returning requested values.
Understanding this, we can finally fix our code.
Now the errorCodes variable contains a materialized list of error codes and we are running the SomeLongRunningOperation only once.
You can test this in practice with the code example from GitHub. The project includes the service and the test classes required to reproduce the scenario.
Next time, we’ll take a look at bad coding patterns (using IEnumerable) that may result in unwanted, multiple instances of classes.
