Skip to content

Conversation

@danscotton
Copy link

Completed the task in the simplest, non-tdd way as possible just to see if I could really – in contrast to my SudokuValidator attempt which was TDD from the get-go. Will attempt to refactor using extract class and extract method and see where it takes me.

… I could really – in contrast to my SudokuValidator. Will attempt to refactor using extract class and extract method and see where it takes me.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you created a hash with a default value of 0 via Hash.new(0), I think you could simplify this line to just o[speaker] += lines.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ben, love this refactoring. I often forget that Ruby allows a hash with default values. Superb!

@r00k
Copy link
Contributor

r00k commented Dec 19, 2013

Congrats on completing the challenge!

I'm looking forward to seeing the solution after some extractions take place. Made one small comment anyhow. :)

@danscotton
Copy link
Author

Thanks @r00k. Love ruby for allowing me to solve it like this, but it's a very different direction than I usually take. I think my SudokuValidator attempt is well designed, but can't help thinking sometimes it's over-engineered. I'm also looking forward to seeing which direction I end up in with this after extracting some roles and interfaces! :)

… more confidence. Scribbled a few ideas down too.
…ass. New unit test for this class, and acceptance test still passing. Ibiblio might take on more responsibilities – maybe parsing the xml and returning an instance of Play.
… to do some dodgy stuff with Aruba to get this running in the same process. Will clean this up afterwards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants