воскресенье, 4 августа 2013 г.

TDD adventures Part 1. Productive TDD from real life.

Following is a real-life demonstration of how effective one may be applying TDD to solve tasks under stress conditions. 
Imagine that you are limited on time, you are under stress and you have a task you've never solved before. Did I mentioned that you don't know the problem domain and your destiny relies upon your solution you'll be judged for? How would you tackle this problem? What would you say If I'd tell you to write a test first? Sounds crazy, eh? You have so many things to do and you don't want to waste your time on testing. Afterwards, maybe... First you have to write... What are you going to write first? Remember, you aren't familiar with problem. Write a test.

Alright, now the problem statement is as follows:
"Compare two CIDR ranges. 
The significant part of IPv4 Address is prefix
255.255.255.255/24 notation means that only the first 24 bits of the IPv4 address make up the network mask. The bits which are not included in the network mask are not important.
There are several ways to write a CIDR range
These representations are equivalent:
255.255.255.255/24
255.255.255.0/24
255.255.255.100/24
Comparing CIDR Ranges
Ranges can be in subset, superset, disjoint or equal to each other. Write a code comparing them." 

- "Fuck, I never knew what this CIDR notation ment. I should have read about it before. Alright, let's google it. Whoooaaa! I'll read through this for half an hour. What do I do? I've already applied TDD in day to day job, I'll give it a try. First things first, I'll write down what I know about the problem. This is going to be my test list."
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • Only the Prefix of the IPv4 Address is Significant;
  • A CIDR Range Has Multiple Representations;
- "That will do for the moment. Which one of them do I pick first? I should pick the simplest to implement it as fast as possible. The first one is an ideal candidate for Starter Test."

    [TestFixture]
    public class CidrRangeTests
    {
        [Test]
        [TestCase("23.45.67.89/16", 23, 16)]
        public void when_instantiated_parses_string_into_address_prefix_and_suffix(string range, byte expectedMostSignificantByte, byte expectedSuffix)
        {
            //arrange 
            //act
            var sut = new CidrRange(range);

            //assert
            Assert.AreEqual(expectedMostSignificantByte, sut.Prefix[0]);
            Assert.AreEqual(expectedSuffix, sut.Suffix);
        }
    }

- "Now that the test is written I am going to write the CidrRange class itself. Forgive me Kent, but I am about to jump over one step - I won't Fake It, I consider this code to be an Obvious Implementation and 'll write it straight away. I had lot's of choclate and I can keep things in mind and I have very short time".

    public class CidrRange
    {
        //auto-props for the sake of simplicity
        public byte Suffix { get; private set; }
        public byte[] Prefix { get; private set; }

        public CidrRange(string range)
        {
            //assert not null
            var parts = range.Split('/');
            //assert count 2
            var prefixParts = parts[0].Split('.');
            //assert count 4
            Prefix = prefixParts.Select(byte.Parse).ToArray();
            Suffix = byte.Parse(parts[1]);
        }
     }

- "Green bar, hooray. Time spent? 10 minutes. OK, whats next? The next simplest test to implement - the shortest way to the green bar, is the second one: Ranges with equal prefix and suffix are equal".


        [Test]
        [TestCase("23.45.67.89/16")]
        [TestCase("1.2.3.4/24")]
        [TestCase("172.84.26.128/16")]
        [TestCase("197.54.16.128/25")]
        public void when_prefix_and_suffix_are_equal_should_consider_other_to_be_equal(string range)
        {
            //arrange 
            var sut = new CidrRange(range);
            var other = new CidrRange(range);

            //act
            //assert
            Assert.AreEqual(sut, other);
        }

- "And again, I triangulate and replace fake with implementation in one step. Remeber, I had lot's of chocolate."


        protected bool Equals(CidrRange other)
        {
            return Suffix == other.Suffix && Prefix.SequenceEquals(other.Prefix);
        }

- "OK, running the tests gives me a green bar. Yeah baby. What do I have to complete?"
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • Only the Prefix of the IPv4 Address is Significant;
  • A CIDR Range Has Multiple Representations;
- "OK, now stop and think. The last one seems to broaden the equality equation for CIDR range. If I put it another way it may sound like the following".
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
- "Cool, I feel for the core of the solution. Now I'll think more about the former: Only the Prefix of the IPv4 Address is Significant, - I may rephrase it like the following:
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
- "OK, so now my test list looks like this":
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
- "Let's code!"

        [Test]
        [TestCase("197.54.16.128/25", "198.54.16.128/25")]
        public void when_significant_prefix_bits_differ_should_not_consider_other_to_be_equal(string leftRange, string rightRange)
        {
            //arrange 
            var sut = new CidrRange(leftRange);
            var other = new CidrRange(rightRange);

            //act
            //assert
            Assert.AreNotEqual(sut, other);
        }

- "Red bar. OK, obviously in order to implement desired behavior I can no longer compare bytes, I have to drill down to bits. What facilities does BCL provides for this? I remember BitConverter and also there is a BitArray class. A-ha! This is what I am going to use, 'cause I don't really want to ressurect my middle age address arithmetic skills. The algorythm is simple: Given the CIDR range 255.255.255.255/24 we know that only the first 24 bits of the IPv4 address make up the network mask, - it means I have to take a prefix, turn it into a bit array, take a suffix, turn it into another bit array starting from the most significant bit and perform a bitwise AND operation in order to get a network mask. After I got both masks, I have to perform an operation, that will tell me if there any difference between them. Well, luckily I had lot's of choclate, and it's obvious that I need to perform a bitwise XOR".


        protected bool Equals(CidrRange other)
        {
            return Suffix == other.Suffix && PrefixesEquals(other);
        }

        private bool PrefixesEquals(CidrRange other)
        {
            var leftMask = new BitArray(Prefix);
            var leftSuffix = new BitArray(32, false);
            for (int i = 31; i >= Suffix; i--)
            {
                leftSuffix[i] = true;
            }
            leftMask = leftSuffix.And(leftMask);

            var rightMask = new BitArray(other.Prefix);
            var rightSuffix = new BitArray(32, false);
            for (int i = 31; i >= other.Suffix; i--)
            {
                rightSuffix[i] = true;
            }
            rightMask = rightSuffix.And(rightMask);

            var result = rightMask.Xor(leftMask);
            return result.Cast().ToArray().All(x => !x);
        }

- "Hmmm, red bar. Something is wrong here."
After five minutes in debugger it appeared that I was creating bit array for mask based on prefix with bytes in reversed order.


    public class CidrRange
    {
        //auto-props for the sake of simplicity
        public byte Suffix { get; private set; }
        public byte[] Prefix { get; private set; }

        public CidrRange(string range)
        {
            //assert not null
            var parts = range.Split('/');
            //assert count 2
            var prefixParts = parts[0].Split('.').Reverse();
            //assert count 4
            Prefix = prefixParts.Select(byte.Parse).ToArray();
            Suffix = byte.Parse(parts[1]);
        }

- "Arghhh, red bar. I'm toast. Come on, keep it on, I've almost found the solution and I've got tests that will prove it. The broken test is simple - Starter test expects to see bytes in reversed order, I'll fix it in a twinkle".


        [Test]
        [TestCase("23.45.67.89/16", 23, 16)]
        public void when_instantiated_parses_string_into_address_prefix_and_suffix(string range, byte expectedMostSignificantByte, byte expectedSuffix)
        {
            //arrange 
            //act
            var sut = new CidrRange(range);

            //assert
            Assert.AreEqual(expectedMostSignificantByte, sut.Prefix[3]);
            Assert.AreEqual(expectedSuffix, sut.Suffix);
        }

- "Finally! Green bar. It took me some time to get here, if I wasn't low on time I'd better be going teeny weeny steps of Fake and Triangulate. OK, now the test list looks like the following".
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
- "Let's code."


        [Test]
        [TestCase("23.45.67.89/16", "23.45.68.00/16")]
        public void when_insignificant_prefix_bits_differ_should_consider_other_to_be_equal(string leftRange, string rightRange)
        {
            //arrange 
            var sut = new CidrRange(leftRange);
            var other = new CidrRange(rightRange);

            //act
            //assert
            Assert.AreEqual(sut, other);
        }

- "red bar, eh? I was supposed to get a green bar!"

After another five minutes in debugger it appeared that I've filled the suffix mask with wrong number of bits and took into account the least significant bits comparing network masks. Fix follows.


        private bool PrefixesEquals(CidrRange other)
        {
            var leftMask = new BitArray(Prefix);
            var leftSuffix = new BitArray(32, false);
            for (int i = 31; i >= 32 - Suffix; i--)
            {
                leftSuffix[i] = true;
            }
            leftMask = leftSuffix.And(leftMask);

            var rightMask = new BitArray(other.Prefix);
            var rightSuffix = new BitArray(32, false);
            for (int i = 31; i >= 32 - other.Suffix; i--)
            {
                rightSuffix[i] = true;
            }
            rightMask = rightSuffix.And(rightMask);

            var result = rightMask.Xor(leftMask);
            return result.Cast().ToArray().Skip(32 - Math.Min(Suffix, other.Suffix)).All(x => !x);
        }

- "A wonderful, beautiful green bar! OK, what do I have?"
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
- "Well, this is not the end, but this was the most interesting part. Following is what is left".
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
  • CIDR range is a subset of other range if prefixes are equal and the suffix is greater;
  • CIDR range is a superset of other range if prefixes are equal and the suffix is less;
  • CIDR range is a disjoint of other range if prefixes differ;
- "I'll take them one by one."

        [Test]
        [TestCase("1.2.3.4/24", "1.2.3.4/16")]
        public void when_prefixes_are_equal_and_left_suffix_is_greater_should_consider_left_to_be_a_subset(string leftRange, string rightRange)
        {
            //arrange 
            var sut = new CidrRange(leftRange);
            var right = new CidrRange(rightRange);

            //act
            var actual = sut.CompareTo(right);
            var expected = sut.Prefix.SequenceEqual(right.Prefix) && sut.Suffix > right.Suffix
                ? RangeIntersectionResult.Subset
                : RangeIntersectionResult.Disjoint;

            //assert
            Assert.AreEqual(expected, actual);
        }

- "Obviously there are more then two results of ranges comparison. There are four of them as follows".


    public enum RangeIntersectionResult
    {
        Equals,
        Subset,
        Superset,
        Disjoint
    }

- "And to compare my range objects I will need another method."


        public RangeIntersectionResult CompareTo(CidrRange other)
        {
            if (Equals(other))
                return RangeIntersectionResult.Equals;

            if (PrefixesEquals(other) && Suffix > other.Suffix)
                return RangeIntersectionResult.Subset;

            return RangeIntersectionResult.Disjoint;
        }

- "Green bar! Hooray."
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
  • CIDR range is a subset of other range if prefixes are equal and the suffix is greater;
  • CIDR range is a superset of other range if prefixes are equal and the suffix is less;
  • CIDR range is a disjoint of other range if prefixes differ;
        [Test]
        [TestCase("172.84.26.128/16", "172.84.26.255/24")]
        public void when_prefixes_are_equal_and_left_suffix_is_less_should_consider_left_to_be_a_superset(string leftRange, string rightRange)
        {
            //arrange 
            var sut = new CidrRange(leftRange);
            var right = new CidrRange(rightRange);

            //act
            var actual = sut.CompareTo(right);
            var expected = sut.Prefix.Skip(1).SequenceEqual(right.Prefix.Skip(1)) && sut.Suffix < right.Suffix
                ? RangeIntersectionResult.Superset
                : RangeIntersectionResult.Disjoint;

            //assert
            Assert.AreEqual(expected, actual);
        }
        public RangeIntersectionResult CompareTo(CidrRange other)
        {
            if (Equals(other))
                return RangeIntersectionResult.Equals;

            if (PrefixesEquals(other) && Suffix > other.Suffix)
                return RangeIntersectionResult.Subset;
            if (PrefixesEquals(other) && Suffix < other.Suffix)
                return RangeIntersectionResult.Superset;

            return RangeIntersectionResult.Disjoint;
        }
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
  • CIDR range is a subset of other range if prefixes are equal and the suffix is greater;
  • CIDR range is a superset of other range if prefixes are equal and the suffix is less;
  • CIDR range is a disjoint of other range if prefixes differ;
        [Test]
        [TestCase("197.54.16.128/25", "197.54.16.127/25")]
        [TestCase("205.00.00.1/32", "205.00.00.00/32")]
        public void when_prefixes_are_different_should_consider_left_to_be_a_disjoint(string leftRange, string rightRange)
        {
            //arrange 
            var sut = new CidrRange(leftRange);
            var right = new CidrRange(rightRange);

            //act
            var actual = sut.CompareTo(right);
            var expected = !sut.Prefix.SequenceEqual(right.Prefix)//this is not exactly correct specification, but it's OK for the sake of simplicity
                ? RangeIntersectionResult.Disjoint
                : RangeIntersectionResult.Subset;

            //assert
            Assert.AreEqual(expected, actual);
        }
  • CIDR range has prefix and suffix parts;
  • Ranges with equal prefix and suffix are equal;
  • when CIDR ranges with exact suffixes differ in insignificant bits, they are considered equal;
  • when CIDR ranges with exact suffixes differ in significant bits, they aren't considered equal;
  • CIDR range is a subset of other range if prefixes are equal and the suffix is greater;
  • CIDR range is a superset of other range if prefixes are equal and the suffix is less;
  • CIDR range is a disjoint of other range if prefixes differ;
- "Time? Wow, I still got some time left. I've manged to complete in under two hours"

This is a real life story disproving the myth of TDD nonproductiveness. Stay tuned as I'll continue my adventures in TDD.

Next time I will refactor this code in a quiet and peacful manner.

P.S. You can find code here.

Комментариев нет: