Seiten

Donnerstag, 24. Dezember 2009

Prime Factors Kata - First Try

This is my first try of the Prime Factors Kata. It's my first Katacast of a kata at all. I performed it with Groovy.

Stefan wrote about katas (in German) a while ago, where he described his experience as an attendee of the contest "TDD with the pros" ("TDD mit den Profis", TmdP) at the XP Days Germany 2009 in Karlsruhe. TmdP is similar to "Programming with the Stars" at Agile 2009 in Chicago, where I was an attendee of.

Stefan was in the final of TmdP with his pair partner Franziska Widmaier. Their task was to calculate all prime factors of a given number, which is the same task as in the Prime Factors Kata. Stefan and Franziska had to finish this task in less than 7 minutes - and they failed to do it in time.

Stefan and I started to work on that kata right after the XP Days ended. Our first shot was in the train from Karlsruhe back to our home town Hamburg. After that we met once via Skype and wrote each step we took during the kata. I practiced a few times per week for the last five weeks or so and showed my performance to Stefan today. He said that it'd might be time to show the result to the world, and here it is:

On Youtube:



I've also shared the Katacast on vimeo and on viddler.

My to-do list for the next try so far:
  • Use a parameterized JUnit test instead of not isolated parameters in a map
  • Use more of TextMate's shortcuts and optimize on a keystroke level
  • Find out why 63 is too big... wtf? [Update: @datenreisender found out that this is related to a bug in Groovy.]
  • [Update: @datenreisender suggested in the comments to use primefactors instead of numbers2primeFactors[number] in the assertion.]
My heartfelt thanks to Stefan, with whom it was a lot of fun preparing that kata!

Happy Christmas!

P.S.: If you want to rate my performance, then post a comment with the number 0 (very bad) to 10 (very good). This kind of rating was inspired by Micah Martin. Any additional feedback is very welcome!

[Update: Stefan Roock made a Kata Bunkai - a commented kata - for this kata. Check it out!]

12 Kommentare:

  1. Why don't you write
      assert primeFactors == generate(number)
    in the test?

    AntwortenLöschen
  2. Jap, seems to me like a good refactoring. Thanks, Marko!

    AntwortenLöschen
  3. Beside the remark from Marko I have two additional ones:
    Between the passing test for the 2 and the one for the 3 you did a lot of refactoring, thus it simply became a matter of weakening the if clause to have the 3 -> [3] pass. I think I would have postponed this refactoring until the test with the 3 passed for the first time.

    Second, I wouldn't have deleted the passing test data for 5, 6, 7, and 8. They were vital information for the algorithm. In production code I would have disabled the next maintenance programmer with vital information about my intentions.

    Overall I liked the video. Did you consider submitting it to Software Craftsmanship Katas?

    AntwortenLöschen
  4. @Markus Gärtner:

    Tests: to delete or not to delete, that's an old question.

    Viewing tests mainly as something driving your design motivates you to delete tests which don't push you to design/programm something new. Viewing tests mainly as regression tests or documentation motivates you to keep them.

    In most projects I tend to view tests as all three, so I say - not surprisingly for some: it depends.

    In this case I too would have kept them, alone because they are so nice and concise.

    AntwortenLöschen
  5. I didn't understand the motivation for some of the steps:

    After the testcase
    assert [2] == generate(2)
    you split up the test method into two test methods before extracting a loop with a map. Why? Why not simply leave one method and extract the map from there?

    BTW: I wouldn't make a constant for the number2PrimeFactors map. I would simply inline it.

    What is the motivation to introduce the "result" parameter even before the testcase for 3?

    When you added the "as integer" it first disturbed me, because I didn't understand why you did it. When reenacting the Kata I discovered that it was needed for the following step, because the division creates a BigDecimal (in my book a Groovy oddity) and % isn't defined for BigDecimal (in my book another Groovy oddity).

    Why did you replace
    number = factor * factor
    in the if clause with
    number % factor == 0
    without a test pushing you to it? Did you consider the former too much a fake?

    AntwortenLöschen
  6. I particularly like that you used failing tests to make sure that your test refactorings were correct.

    AntwortenLöschen
  7. @Markus: "Between the passing test for the 2 and the one for the 3 you did a lot of refactoring, thus it simply became a matter of weakening the if clause to have the 3 -> [3] pass. I think I would have postponed this refactoring until the test with the 3 passed for the first time."

    One could do that, but I don't see the benefit. And I don't see the change from number != 2 to number == 1 as a "weakening the if clause". How would it change things if I'd have done the refactoring after the test for 3 and not after the test for 2?

    You wrote "I wouldn't have deleted the passing test data for 5, 6, 7, and 8."

    I could have left the numbers from 5 to 8 in the test. I wouldn't have been a new test (because it's not red), but it would have been a refactoring to the tests. However, they are redundant: they are all in already tested equivalence classes! 5 and 7 are prime like 2 and 3 are, so they wouldn't provide more information for a maintainer, but they would blur my intention expressed via the other test data (like 2 and 3). 6 and 8 are divisibly by other prime factors, like 4 is (which was already in the tests before).

    See? No new driving tests (no red) and redundancy. That's why I delete those tests right away.

    You wrote "Overall I liked the video. Did you consider submitting it to Software Craftsmanship Katas?"

    Thank you :) Yes, actually I wrote an email this morning to them. Would be great if they would write about it :)

    AntwortenLöschen
  8. @Marco: "In this case I too would have kept them, alone because they are so nice and concise."

    Nope, I don't see that as a reason to keep them. Even if things are nice and concise, they have to be necessary. Otherwise it'd be valid to keep dead code because of death's beauty. That's not an option for me.

    AntwortenLöschen
  9. @Marko: Sorry for the late reply. Holidays are too busy :)

    Thanks for your great feedback questions! Let me try to answer them.

    You wrote "you split up the test method into two test methods before extracting a loop with a map. Why?"

    To move an assertion you have to make it red. That's what I've done before I moved the assertion into the closure.

    Now, if you have two red assertions only the first one would be executed (and evaluated to red), and the second one would never be reached by the execution flow. That would mean that you could "lose" the second assertion by accident, and no test would inform you about it. That's why I first separated the two assertion from each other.

    You wrote in the same context "Why not simply leave one method and extract the map from there?"

    That would mean, as far as I understand your intention and with the premise, that I can't move more than a single assertion at one time, so that would mean, that I'd get rid of one of the assertions. But they were both worth to keep, since they cover the ground my production code stands on. I only want to delete redundant and not-driving tests, so I'd like to keep both of the assertions.

    Do you know a green path where you could extract the map out of the first test with both assertions in place?

    You wrote further in the same context "BTW: I wouldn't make a constant for the number2PrimeFactors map. I would simply inline it. "

    I thought about that. The way I look at it is, that the data itself is not part of the fixture. Therefor there's no need to have it as a instance variable or as a constant. On the other hand, having the data inside of my test method would bloat the method to an amount that I would not like to have. Another point could be, that the data and how it's used are a little bit more separated in a constant for the data and a method for the data usage, and that's, well, more tidy.

    I suppose there's a principle behind this, but it's not yet within my reach right now.

    You wrote "What is the motivation to introduce the "result" parameter even before the testcase for 3?"

    These are actually two steps in one (yes, I know, shame on me :) ). Before the test case for input 3 there are two empty lists [] in the production code. I could eliminate them with an extraction and I would have "def result = []", However, I could save the "def" with the parameter solution. And, yes, I have to admit, I knew the solution for the further test steps, so I saved the variable and wrote down the parameter shamelessly.

    You wrote "Why did you replace
    number = factor * factor
    in the if clause with
    number % factor == 0
    without a test pushing you to it? Did you consider the former too much a fake?
    "

    Nope, no fake, but not DRY. There are two times "factor" in "number = factor * factor", and that's my motivation to search for another way, a DRY way.

    There's one thing that might be worth to be written down: You asked about the motivation for the result parameter, the "as integer" and the "number % factor == 0". Those are refactorings considering further test cases. When practicing a kata, one will run in several red tests without a clue how to make it green with only one or a few changes. Then one will remove the red test (or comment it out), will refactor, will write the red test again, and now one might be able to make it green with a simple change. In the end, when one will record the kata cast, observers only see the result, the clean and green way from start to end, but not the tiny little steps of failure which are necessary to achieve it.

    That's the long answer from my point of view, why some of my step's motivations might not be obvious for observers.

    AntwortenLöschen
  10. @Alex: Yes, I took especially care of the part with the red assertions. Nice from you to mention it :)

    AntwortenLöschen
  11. Guys, thanks for all the feedback and comments! I'm sitting here with a big smile in my face and am happy :) Happy New Year to all of you and to your families!

    AntwortenLöschen
  12. This code kata in bunkai style: http://stefanroock.wordpress.com/2010/01/07/code-kata-bunkai-prime-factors/

    AntwortenLöschen