Another Approach to the Diamond Kata
I saw that Alistair Cockburn had written a post about Seb Rose’s post on the Diamond Kata. I only read the beginning of both of those because I recognized the problem that Seb described with the “Gorilla” approach that, upon reaching the ‘C’ case.
“The code is now screaming for us to refactor it, but to keep all the tests passing most people try to solve the entire problem at once. That’s hard, because we’ll need to cope with multiple lines, varying indentation, and repeated characters with a varying number of spaces between them.”
I’ve run into such situations before, and it’s always be a clue for me to back up and work in smaller steps. Seb describes that the ‘B’ case, “easy enough to get this to pass by hardcoding the result.” Alistair describes the strategy as “shuffle around a bit” for the ‘B’ case. I’m not sure what “shuffling around a bit” means and I don’t think it would be particularly easy to get both ‘A’ and ‘B’ cases working with constants and not heading down a silly “if (letter == 'A') "¦ elseif (letter == 'B') "¦
” implementation. I was curious how I would approach it, and decided to try. (Ron Jeffries also wrote a post on the topic.) I didn’t read any of these three solutions before implementing my own, just so I could see what I would do.
If you don’t want to read the details, you can skip to the conclusion.
Step 1: Making sure rspec is working
Like Ron, I often write a dummy test first to make sure things are hooked up right. This was especially true since I haven’t programmed in Ruby for awhile, so long that I haven’t worked with Ruby 2 or Rspec 3. It seemed like a good time to get familiar with these.
require 'rspec' describe "setup" do it "can call rspec" do expect(2).to eql(2) end end
Of course, I originally had “expect(1).to eql(2)
” to make the test fail. Once I got the syntax right and installation correct, I had a failing test and then changed it to make it pass.
Step 2: Trivial representation of degenerate diamond
Now I start in earnest, taking care of the trivial case.
describe Diamond do describe '.create(A)' do subject { Diamond.create('A') } it "has a trivial representation" do expect(subject.representation).to eql "A\n" end end end
is accomplished with
class Diamond def self.create(max_letter) Diamond.new(max_letter) end def initialize(max_letter) end def representation "A\n" end end
There are several design choices in this. I chose a factory method because it seemed more readable in the test. It delegated to the constructor so I’d have an instance for expressing expectations. And, of course, as Seb and Alistair and Ron all expect, I fake the return with a constant. Easy-peasy!
Step 3: Output all the necessary lines
The next step is obviously to implement the ‘B’ case. Hmmm”¦ Seb suggested faking it with a constant, also, but that immediately leads me down the path of an “if (letter == 'A') "¦ elseif (letter == 'B') "¦
” implementation. I don’t mind faking a return, but I don’t want my program to look like What the Tortoise Said to Achilles. I find I have to insert logic before I reach the “solve the whole problem at ‘C'” point predicted by Seb and Alistair. If I write a test for the representation, I need to solve the whole problem for the ‘B’ case, and that’s too big a step for me. It’s got calculations and formatting all at once, and that’s too much for me to hold in my brain at once.
I decided to start with outputting the right number of lines.
describe '.create(B)' do subject { Diamond.create('B') } it "has three lines in the representation" do expect(subject.representation.lines.count).to eql 3 end end
I need a line for each letter from ‘A’ up to the maximum letter, and then back down to ‘A’ without a duplicate line for the maximum letter. I ended up with this:
def initialize(max_letter) @letters= ('A' .. max_letter).to_a end def representation output= "" @letters.each { |letter| output << letter+"\n" } @letters.reverse[1..-1].each { |letter| output << letter+"\n" } output end
Saving the array of letters rather than the max_letter seemed an easy way to count, and I would need the value of the current letter, anyway. In retrospect, that was a bit of speculation. The current test doesn’t check the letters in the output.
I initially wanted to walk up the array of letters and back down. In the C language that would have been simple, but it wasn’t convenient in Ruby. Reversing the array, minus the max_letter, let me conveniently use the ‘each’ idiom. I avoided numerical calculations and I think this will work all the way up to the ‘Z’ case. I check it for the ‘C’ case.
describe '.create(C)' do subject { Diamond.create('C') } it "has five lines in the representation" do expect(subject.representation.lines.count).to eql 5 end end
Everything works fine.
Taking stock of where we are
Let’s take a look now and see what we have. I haven’t described the non-working changes I’ve made in each of these steps. I’d be embarrassed at all the silly things I typed that didn’t compile. (I did learn that you can’t reverse the subscripts of an array to get a slice in the other direction.) I also haven’t described how the code looked before making simplifying refactorings.
As I look at this code, I’m more aware that I’m outputting the correct letter in each row, even though I don’t have a test for this. No matter, that test will come. I’m more unhappy with the duplication between the ‘describe’ and ‘subject’ lines in the tests. There’s surely a way to avoid this duplication, but I didn’t find it in a 10-minute search of the web. I decided to let it be for now. It’s only a kata, and by publishing something “wrong,” people will surely tell me how to do it right. This is the first time I’ve used the explicit “subject” idiom and it’s still unfamiliar to me.
Step 4: handle indentation of early lines
Formatting the entire line still seemed like a big step to me. Perhaps it would have been less daunting had I done a lot of up-front thinking about the problem as Alistair did. Instead, I just toddled along at my own pace. If I figured out how many spaces went at the beginning of each line, that should help me figure out how many spaces go in the middle and end. Ignoring those later problems, I added a new expectation for the ‘B’ case.
describe '.create(B)' do subject { Diamond.create('B') } it "has three lines in the representation" do expect(subject.representation.lines.count).to eql 3 end it "indents the first line" do expect(subject.representation.lines[0]).to start_with "_A" end end
I almost started calculating the number of spaces required, but I noticed that, in addition to each row being dedicated to a letter, so was each column. Like iterating through the rows, I could iterate through the columns.
def representation output= "" @letters.each { |letter| @letters.reverse.each { |position| character= (position == letter) ? letter : '_' output << character } output << "\n" } @letters.reverse[1..-1].each { |letter| output << letter+"\n" } output end
Again, this code looks like it will work all the way to the ‘Z’ case, so I add a couple checks for the ‘C’ case.
it "indents the first line" do expect(subject.representation.lines[0]).to start_with "__A" end it "indents the second line" do expect(subject.representation.lines[1]).to start_with "_B_" end
The code seems a little messy. I’ve got a doubly nested loop with a calculation in the middle. I decided it would read more clearly if I extracted the calculation.
def either_letter_or_blank (position, letter) (position == letter) ? letter : '_' end def representation output= "" @letters.each { |letter| @letters.reverse.each { |position| output << either_letter_or_blank(position, letter) } output << "\n" } @letters.reverse[1..-1].each { |letter| output << letter+"\n" } output end
Note that only the first half of the diamond is being formatted. I’ll get to the latter half, but it’ll probably be simpler if I format the whole line, first. Otherwise I’ll have two places to fix that formatting. I proceed to “¦
Step 5: handle filling out early lines
Let’s add spaces to the ends of the lines. For the ‘B’ case, that means”¦
it "fills out the first line" do expect(subject.representation.lines[0]).to end_with "A_\n" end
Walking the reversed array worked so well for going down the later rows, let’s do the same for the later columns.
def representation output= "" @letters.each { |letter| @letters.reverse.each { |position| output << either_letter_or_blank(position, letter) } @letters[1..-1].each { |position| output << either_letter_or_blank(position, letter) } output << "\n" } @letters.reverse[1..-1].each { |letter| output << letter+"\n" } output end
That’s pretty ugly, isn’t it. Having two inner loops within the first of two outer loops is nuts, especially since it’ll have to be done again in the second outer loop. Let’s extract another method.
def line_for_letter (letter) line= "" @letters.reverse.each { |position| line << either_letter_or_blank(position, letter) } @letters[1..-1].each { |position| line << either_letter_or_blank(position, letter) } line << "\n" end def representation output= "" @letters.each { |letter| output << line_for_letter(letter) } @letters.reverse[1..-1].each { |letter| output << letter+"\n" } output end
We’re almost done. The descending rows are now trivial. I take a big step and specify the entire ‘B’ case output.
it "outputs the correct diamond" do expected= "_A_\n"+ "B_B\n"+ "_A_\n" expect(subject.representation).to eql expected end
And make it pass with a single line change.
def representation output= "" @letters.each { |letter| output << line_for_letter(letter) } @letters.reverse[1..-1].each { |letter| output << line_for_letter(letter) } output end
I think I’m done. Let’s check the ‘C’ case.
it "outputs the correct diamond" do expected= "__A__\n"+ "_B_B_\n"+ "C___C\n"+ "_B_B_\n"+ "__A__\n" expect(subject.representation).to eql expected end
Yep, it works as expected. Were this a production app, I’d do some work to protect against bad input. I’d also hook it up to the command line, as Seb initially described. Oh, and remove that original “setup” spec. The full code is below, and in GitHub.
The complete spec
require 'rspec' require_relative './diamond' describe "setup" do it "can call rspec" do expect(2).to eql(2) end end describe Diamond do describe '.create(A)' do subject { Diamond.create('A') } it "has a trivial representation" do expect(subject.representation).to eql "A\n" end end describe '.create(B)' do subject { Diamond.create('B') } it "has three lines in the representation" do expect(subject.representation.lines.count).to eql 3 end it "indents the first line" do expect(subject.representation.lines[0]).to start_with "_A" end it "fills out the first line" do expect(subject.representation.lines[0]).to end_with "A_\n" end it "outputs the correct diamond" do expected= "_A_\n"+ "B_B\n"+ "_A_\n" expect(subject.representation).to eql expected end end describe '.create(C)' do subject { Diamond.create('C') } it "has five lines in the representation" do expect(subject.representation.lines.count).to eql 5 end it "indents the first line" do expect(subject.representation.lines[0]).to start_with "__A" end it "indents the second line" do expect(subject.representation.lines[1]).to start_with "_B_" end it "outputs the correct diamond" do expected= "__A__\n"+ "_B_B_\n"+ "C___C\n"+ "_B_B_\n"+ "__A__\n" expect(subject.representation).to eql expected end end end
The complete code
class Diamond def self.create(max_letter) Diamond.new(max_letter) end def initialize(max_letter) @letters= ('A' .. max_letter).to_a end def either_letter_or_blank (position, letter) (position == letter) ? letter : '_' end def line_for_letter (letter) line= "" @letters.reverse.each { |position| line << either_letter_or_blank(position, letter) } @letters[1..-1].each { |position| line << either_letter_or_blank(position, letter) } line << "\n" end def representation output= "" @letters.each { |letter| output << line_for_letter(letter) } @letters.reverse[1..-1].each { |letter| output << line_for_letter(letter) } output end end
There are some interesting things I notice by comparing my solution with Seb’s and Alistair’s and Ron’s approaches.
Alistair starts with a primitive at the inside of the problem, making rows and then padding them to size. I don’t know whether thinking deeply about the problem leads to starting with a primitive, or if starting with a primitive requires you to think deeply before you start coding. I do think there’s a connection between the two.
I don’t think that connection is unbreakable. I’ve started with a low-level primitive when I was trying to see what I could do with some example data, or with a library that was new to me. Once I got a clue what I could do, then I found it easier to switch gears and work outside in, story-test first, and work my way back down to that primitive. I found it interesting that sometimes I would change the primitive subtly when I did that.
Seb started with the concept of what letters should appear, and in what order, and then made modifications to get them into the right format. I think that’s what lead him to modify and reuse his tests. They were specifying things that were only temporarily true.
I find it interesting how Seb approached the output like a sentence or paragraph. That never occurred to me. I viewed it as a two-dimensional shape, which would never lead me to the series of tests that he used. Seb’s approach is unique among the four in that is doesn’t pad the right side of the rows.
Ron started with building rows and calculating how many spaces they should have. This lead him down a path of lots of calculations. Along the way, he made the simplifying discovery that the four quadrants were symmetrical, saving some calculations.
From this, I conclude that there are certainly many ways to approach a given problem. The way you conceive of the problem and approach the solution has a great effect on the result. If you think you need to think things out before you start coding, then you probably will, and that may affect the solution you achieve. Sometimes it’s hard to let go of our ideas and be open to the opportunities presented by the code.
Certainly Ron is right that it’s helpful to think about things all the way through the implementation.
[Dec 2, 2014 — fixed some mangled formatting of the code.]
Very nice indeed. Here’s a trace of some thoughts as I read it:
I had to look ahead to see what @letters was. I think its definition didn’t appear till later.
the ABC CDB column / row notion didn’t come to me at once. I tried explaining it in words in my notes and my words helped but not enough to recommend them.
The observation ABC CBA is brilliant and non-obvious IMO. As such it needs some hint to the reader.
A pair consisting of you and me would likely have found an even better solution sooner, because I found it later based on yours. The power of two is amazing to me, always.
While still in Step 4, I was thinking “if ABC CBA works, then so will ABCBA CBABC”. that insight collapses the problem.
The “@letters.reverse” and “@letters.reverse[1,-1]” made me stumble. Maybe if I were more facile with Ruby they’d have been more clear. As it stands, I’d want to do something to clarify them.
It’s interesting that the row logic is letters then letters reverse and the line logic is letters reverse then letters.
It’s a lovely solution. To me it shows the potential value of thinking about the 2-D problem, while my second one shows the pitfalls.
Yours inspired mine, also tweeted:
rows = “ABCDCBA”
cols = “DCBABCD”
rows.chars { |r|
l = “”
cols.chars {| c|
l << ((r==c)?r:"-")
}
puts l
}
Very sweet. Thanks!
Early typo in the above: ABC CBA not ABC CXY or whatever I said 🙂
Ron, I tried to write idiomatic Ruby. I agree that “@letters.reverse” and “@letters.reverse[1,-1]” would likely be more obvious were you a more frequent Ruby programmer.
And, yes, I agree that we’d likely achieve a better solution earlier if we’d paired.