Today I was refactoring some unit tests, trying to get rid of some funky smelling tests, I made the all to common mistake of trying to be too clever.
The smell
This particular project has several entities that have a Timestamp field, represented by a byte array in the domain objects - err, code... depending the semantics you use. What I found to be particularly stinky was the inline byte arrays that were scattered all through the test code. And what’s worse, most of the arrays weren’t even of the correct size!
For the record, they should be 8-bytes long... byte[] timestamp = new byte[8];
Time to... Refactor!
To clean it up a bit I decided to introduce a test helper method that would generate a pseudo-random byte array of the correct size. To make the method even more generic, and theoretically more useful, I decided to delegate the real work to yet another method, GenerateRandomByteArray.
Oh, and I also used a couple of other methods from our code base to keep the code short. One just gets a random integer and the other allows me execute an Action delegate on each item of an IEnumerable collection, kinda' like a code block in Ruby... more on that in a future blog post. :)
1: public static byte[] GenerateRandomTimestamp()
2: {
3: return GenerateRandomByteArray(8);
4: }
5:
6: public static byte[] GenerateRandomByteArray(int len)
7: {
8: int i = UnitTestHelper.GetRandomInt(99);
9:
10: byte[] timestamp = new byte[len];
11: timestamp.Each(b => b = (byte)i++);
12:
13: return timestamp;
14: }
Anyhow, the code certainly compiles, but we all know that don’t mean jack! The method always returns a byte array where every byte = 0. So that’s hardly random.
Its a Value-Type, dummy!
The problem is with the Lambda expression in line 11. The Each is iterating over the values of the byte array, so the parameter for each iteration, b, is just byte. And in the .net world, and probably most others, a byte is value-typed.
Meaning? Meaning the scope of changes made to the parameter are limited to the statement block on the right-hand side of the => token.
The answer: use a closure to update the value directly inside the byte array itself.
1: public static byte[] GenerateRandomByteArray(int len)
2: {
3: int i = UnitTestHelper.GetRandomInt(99);
4:
5: byte[] timestamp = new byte[len];
6: timestamp.Each(b =>
7: {
8: len--;
9: timestamp[len] = (byte) i++;
10: });
11:
12: return timestamp;
13: }
Lessons learned?
- Don’t be clever for clever’s sake!
- Value-Types are value-types are value types.
- Closures rock the Casaba!
Technorati Tags:
CSharp,
lambda,
closure,
Ruby