sliming and refactoring and deliberate duplication



Suppose I'm doing the Print-Diamond kata in Ruby:
Given a letter print a diamond starting with 'A' 
with the supplied letter at the widest point. 
For example: print-diamond 'E' prints
    A
   B B
  C   C
 D     D
E       E
 D     D
  C   C
   B B
    A
I start with a test
def test_diamond_A
  assert_equal ['A'], diamond('A')
end
which I pass using
def diamond(widest)
  ['A']
end
I add another test
def test_diamond_B
  assert_equal [' A',
                'B B',
                ' A'], diamond('B')
end
which I pass using
def diamond(widest)
  if widest == 'A'
    return ['A']
  end
  if widest == 'B'
    return [' A',
            'B B',
            ' A']
  end
end
I add one more test
def test_diamond_C
  assert_equal ['  A',
                ' B B',
                'C   C',
                ' B B',
                '  A'], diamond('C')
end
which I pass using
def diamond(widest)
  if widest == 'A'
    return ['A']
  end
  if widest == 'B'
    return [' A',
            'B B',
            ' A']
  end
  if widest == 'C'
    return ['  A',
            ' B B',
            'C   C',
            ' B B',
            '  A']
  end
end
The tests have already proved valuable:
  • I've decided I don't want to actually test printing
  • I've chosen the result format - an array of strings
  • I've chosen not to embed newlines at the end of the strings
  • I've something to refactor against
However, there is no point in carrying on sliming. As the tests get more specific, the code should get more generic. I have three specific tests, but the code is equally specific. I need to generalize the code.

While coding the array of strings for the 'C' case I found myself copying the result for 'B' and modifying that. Specifically, I had to:
  • duplicate the 'B B' string
  • add a space at the start of the ' A' and 'B B' strings
  • add a new middle string 'C C'
This gave me the idea to try a recursive implementation. My first step was to refactor the code to this:
def diamond(widest)
  d = inner_diamond(widest)
  mid = d.length / 2
  d[0..mid-1] + d[mid+1..-1]
end

def inner_diamond(widest)
  if widest == 'A'
    return ['A',
            'A']
  end
  if widest == 'B'
    return [' A',
            'B B',
            'B B',
            ' A']
  end
  if widest == 'C'
    return ['  A',
            ' B B',
            'C   C',
            'C   C',
            ' B B',
            '  A']
  end
end
This looks a promising step towards a recursive solution - to make the implementation of 'C' contain the implementation of 'B' and then add strings only for 'C'. So, remembering what I had to do when copying and modifying, I refactored to this:
def inner_diamond(widest)
  if widest == 'A'
    return ['A',
            'A']
  end
  if widest == 'B'
    return [' A',
            'B B',
            'B B',
            ' A']
  end
  if widest == 'C'
    b = inner_diamond('B')
    upper,lower = split(b.map{ |s| ' ' + s })
    c = widest + '   ' + widest
    return upper + [c,c] + lower
  end
end

def split(array)
  mid = array.length / 2
  [ array[0..mid-1], array[mid..-1] ]
end
From here I verified the recursive solution works for 'B' as well:
def inner_diamond(widest)
  if widest == 'A'
    return ['A',
            'A']
  end
  if widest == 'B'
    a = inner_diamond('A')
    upper,lower = split(a.map{ |s| ' ' + s })
    b = widest + ' ' + widest
    return upper + [b,b] + lower
  end
  if widest == 'C'
    b = inner_diamond('B')
    upper,lower = split(b.map{ |s| ' ' + s })
    c = widest + '   ' + widest
    return upper + [c,c] + lower
  end
end
Now I worked on generalizing the use of the hard-coded argument to inner_diamond() and the hard-coded number of spaces:
def inner_diamond(widest)
  if widest == 'A'
    return ['A','A']
  end
  if widest == 'B'
    a = inner_diamond(previous(widest))
    upper,lower = split(a.map{ |s| ' ' + s })
    n = (widest.ord - 'A'.ord) * 2 - 1
    b = widest + (' ' * n) + widest
    return upper + [b,b] + lower
  end
  if widest == 'C'
    b = inner_diamond(previous(widest))
    upper,lower = split(b.map{ |s| ' ' + s })
    n = (widest.ord - 'A'.ord) * 2 - 1
    c = widest + (' ' * n) + widest
    return upper + [c,c] + lower
  end
end

def previous(letter)
  (letter.ord - 1).chr
end
Now I collapsed the duplicated specific code to its more generic form:
def inner_diamond(widest)
  if widest == 'A'
    return ['A','A']
  else
    a = inner_diamond(previous(widest))
    upper,lower = split(a.map{ |s| ' ' + s })
    n = (widest.ord - 'A'.ord) * 2 - 1
    b = widest + (' ' * n) + widest
    return upper + [b,b] + lower
  end
end
Finally some renaming:
def inner_diamond(widest)
  if widest == 'A'
    return ['A','A']
  else
    inner = inner_diamond(previous(widest))
    upper,lower = split(inner.map{ |s| ' ' + s })
    n = (widest.ord - 'A'.ord) * 2 - 1
    middle = widest + (' ' * n) + widest
    return upper + [middle,middle] + lower
  end
end
To summarise:
  • When sliming I try to think ahead and choose tests which allow me to unslime the slime.
  • If I have slimed 3 times, my next step should be to unslime rather than adding a 4th gob of slime.
  • My first unsliming step is often deliberate duplication, done in a way that allows me to collapse the duplication.


1 comment:

  1. Jon, I just did this kata today on cyber-dojo and I'm glad I forgot this post :-) It was fun for me and my pair to reason about this and come up with our own solution. I want to try this recursive approach next!

    ReplyDelete