Skip to content
This repository was archived by the owner on Jun 8, 2019. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1c390ad
Project setup. Adds first passing scenario.
danscotton Dec 3, 2013
faad0b5
Adds new failing scenario. Started to flesh out some objects and thei…
danscotton Dec 3, 2013
060ed25
Adds Sudoku::FileParser with spec.
danscotton Dec 3, 2013
37e1af6
Adds Sudoku::Puzzle and Sudoku::Row specs with simple implementations…
danscotton Dec 5, 2013
8f3f42a
Move Puzzle and Row out to their own files. Refactor the specs a litt…
danscotton Dec 6, 2013
df3a0dc
Adds Column class along with :columns message for Puzzle. Utilises Ma…
danscotton Dec 6, 2013
cf3a99d
Refactored Row and Column objects to utilise new Unit module. Refacto…
danscotton Dec 8, 2013
17dea6d
Adds complete? method to Puzzle.
danscotton Dec 8, 2013
d0f951f
Amend FileParser to convert strings into numbers.
danscotton Dec 8, 2013
18bd9c8
Passing second scenario – replaced stub Puzzle with real implementation.
danscotton Dec 8, 2013
a260fd7
Finalise feature with two remaining scenarios passing. Removed the Su…
danscotton Dec 8, 2013
ce92e3d
Refactor to remove overuse of Sudoku module. Done the same in the spe…
danscotton Dec 8, 2013
c1797a1
Adds new error listing requirements to the feature. Refactored out a …
danscotton Dec 9, 2013
dd4a140
Adds new spec describing error output for rows. Feature still failing…
danscotton Dec 9, 2013
21c2e69
Puzzle's :rows, :columns and :boxes return hashes indexed from 1, ins…
danscotton Dec 9, 2013
47f941c
Complete feature with error listings and their positions.
danscotton Dec 10, 2013
bfc9f7d
Some small tinkerings and refactorings. Bit more checking around the …
danscotton Dec 10, 2013
9b821e7
Fix truthy errors bug as suggested by JoelQ.
danscotton Dec 12, 2013
3ac7e3c
Remove :file accessor from FileParser.
danscotton Dec 18, 2013
68e5126
Refactor FileParser to use nice :split method with regex as suggested…
danscotton Dec 22, 2013
e17fd1f
Small refactor symbol to proc – lovely stuff.
danscotton Dec 22, 2013
013686d
Great refactoring from joelq, simplifying Puzzle's :all_units method.
danscotton Dec 22, 2013
604ac80
Couple of small refactorings in Unit from joelq
danscotton Dec 22, 2013
6044015
Simplify Unit by removing module and BaseUnit class and just keeping …
danscotton Dec 22, 2013
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tmp/
3 changes: 3 additions & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
--color
--format documentation
--require spec_helper
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
source 'https://rubygems.org'
gemspec
45 changes: 45 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
PATH
remote: .
specs:
sudoku_validator (0.0.1)

GEM
remote: https://rubygems.org/
specs:
aruba (0.5.3)
childprocess (>= 0.3.6)
cucumber (>= 1.1.1)
rspec-expectations (>= 2.7.0)
builder (3.2.2)
childprocess (0.3.9)
ffi (~> 1.0, >= 1.0.11)
cucumber (1.3.10)
builder (>= 2.1.2)
diff-lcs (>= 1.1.3)
gherkin (~> 2.12)
multi_json (>= 1.7.5, < 2.0)
multi_test (>= 0.0.2)
diff-lcs (1.2.5)
ffi (1.9.3)
gherkin (2.12.2)
multi_json (~> 1.3)
multi_json (1.8.2)
multi_test (0.0.2)
rake (10.1.0)
rspec (2.14.1)
rspec-core (~> 2.14.0)
rspec-expectations (~> 2.14.0)
rspec-mocks (~> 2.14.0)
rspec-core (2.14.7)
rspec-expectations (2.14.4)
diff-lcs (>= 1.1.3, < 2.0)
rspec-mocks (2.14.4)

PLATFORMS
ruby

DEPENDENCIES
aruba
rake
rspec
sudoku_validator!
9 changes: 9 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
require 'cucumber'
require 'cucumber/rake/task'

Cucumber::Rake::Task.new(:features) do |t|
t.cucumber_opts = "features --format pretty -x"
t.fork = false
end

task :default => :features
24 changes: 24 additions & 0 deletions bin/sudoku-validator
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env ruby

require_relative '../lib/sudoku_fileparser'
require_relative '../lib/sudoku_puzzle'
require_relative '../lib/sudoku_validator'

exit unless filename = ARGV[0]

filepath = File.expand_path(filename)
if File.exist?(filepath)
parser = Sudoku::FileParser.new(File.read(filepath))
puzzle = Sudoku::Puzzle.new(parser.parse_rows)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if Sudoku::FileParser#parse_rows returned a Sudoku::Puzzle object rather than an array of arrays? That would eliminate this intermediate state. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I like that. At the time I was thinking along the lines of, "What if we needed a Sudoku::JSONParser?" and so thought I needed a common format between the parser and the Puzzle, but you're right – any object that implements the 'Parser' interface/role could always return a Puzzle object.

In terms of testing this with RSpec, would you use expect_any_instance_of(Sudoku::Puzzle).to receive(:new)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@danscotton I agree, all of the parsers could return Puzzle instances.

As far as testing, I would probably do something like expect(parser.parse).to eq my_puzzle. This will work because you re-defined Puzzle#== to check for value rather than identity.

Copy link
Author

Choose a reason for hiding this comment

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

That's true, thanks. If my Puzzle object wasn't a value object, would you go down the any_instance_of route then? I'm sure I've read something from thoughtbot about that being a bit of a smell but I'm not sure. I'll ask it in the forums.

validator = Sudoku::Validator.new(puzzle)

puts validator.status
if validator.errors.any?
puts "Errors:"
validator.errors.each.with_index(1) do |error, index|
puts " #{index}) #{error}"
end
end
else
puts "Could not find file #{filename}"
end
23 changes: 23 additions & 0 deletions features/sudoku_validator.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
Feature: Validating .sudoku files

Scenario: A valid, complete sudoku
When I successfully run `sudoku-validator ./valid_complete.sudoku`
Then the output should contain "This sudoku is valid."
Then the output should not contain "Errors:"

Scenario: A valid, incomplete sudoku
When I successfully run `sudoku-validator ./valid_incomplete.sudoku`
Then the output should contain "This sudoku is valid, but incomplete."
Then the output should not contain "Errors:"

Scenario: An invalid, complete sudoku
When I successfully run `sudoku-validator ./invalid_complete.sudoku`
Then the output should contain "This sudoku is invalid."
Then the output should contain "Column 4 contains a duplicate 8 in squares 2 and 5"
Then the output should contain "Column 6 contains a duplicate 2 in squares 1 and 5"

Scenario: An invalid, incomplete sudoku
When I successfully run `sudoku-validator ./invalid_incomplete.sudoku`
Then the output should contain "This sudoku is invalid."
Then the output should contain "Column 2 contains a duplicate 5 in squares 1 and 7"
Then the output should contain "Column 5 contains a duplicate 8 in squares 2 and 7"
5 changes: 5 additions & 0 deletions features/support/env.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'aruba/cucumber'

Before do
@dirs = ["."]
end
28 changes: 28 additions & 0 deletions lib/sudoku_fileparser.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module Sudoku
class FileParser
def initialize(file = nil)
raise(ArgumentError, "A file is required.") if file.nil?
@file = file
end

def parse_rows
@file.strip.each_line.map do |line|
parse_line(line) unless separator?(line)
end.compact
end

private

def parse_line(line)
parse_empties(line).split(/[^\d]+/).map(&:to_i)
end

def parse_empties(line)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for small well-named private methods here. Makes the source very readable.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

line.gsub(".", "0")
end

def separator?(line)
line.start_with?('-')
end
end
end
57 changes: 57 additions & 0 deletions lib/sudoku_puzzle.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
require_relative 'sudoku_unit'
require 'matrix'

module Sudoku
class Puzzle
def initialize(rows = [])
raise(ArgumentError, "An array of rows is required to create a puzzle.") if rows.empty?
@grid = create_grid(rows)
end

def rows
Hash[@grid.row_vectors.map.with_index(1) do |row, index|
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of Hash[] here. Personally, I like to keep the argument to Hash[] as simple as possible. I would extract the building of the array into a private method or at least a local variable.

I notice that building an array of rows and an array of columns is almost identical. This is probably a good candidate for a private method. Something like:

def rows
  Hash[build_units(@grid.row_vectors, Row)]
end

def columns
  Hash[build_units(@grid.column_vectors, Column)]
end

private

def build_units(vectors, unit_class)
  vectors.map.with_index(1) do |unit, index|
    [index, unit_class[*unit]]
  end
end

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I thought about this. The only thing that stopped me doing it was how rows and columns would be utilising this private method, but boxes wouldn't because of it's complexity. Maybe it's just about trying to find the right name for the private method? I like build_units, but again in my head I hear..."well, does that mean boxes is not building units?".

I like the Hash[] syntax too, but it does look ugly around these blocks. Your suggestion does make it seem much nicer. I'll mull it over and see where we end up ;)

[index, Row[*row]]
end]
end

def columns
Hash[@grid.column_vectors.map.with_index(1) do |column, index|
[index, Column[*column]]
end]
end

def boxes
Hash[[0, 3, 6].repeated_permutation(2).map.with_index(1) do |(x, y), index|
[index, Box[*@grid.minor(x, 3, y, 3).to_a.flatten]]
end]
end

def complete?
rows.all? { |index, row| row.complete? }
end

def valid?
all_units.all?(&:valid?)
end

def eql?(other)
hash == other.hash
end

def hash
@grid.hash
end

alias_method :==, :eql?

private

def create_grid(input_rows)
Matrix[*input_rows]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Nice use of the Matrix class here

end

def all_units
rows.values + columns.values + boxes.values
end
end
end
59 changes: 59 additions & 0 deletions lib/sudoku_unit.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
module Sudoku
class Unit
def self.[](*values)
new(values)
end

def initialize(values)
@squares = values
end

def positions
@squares.each.with_index(1).inject ({}) do |h, (number, position)|
h.tap { |h| h[number] = h[number] ? [h[number]].flatten << position : position }
end
end

def valid?
non_empty_squares.uniq.count === non_empty_squares.count
end

def complete?
@squares.none?(&:zero?)
end

def eql?(other)
self.class == other.class && hash == other.hash
end

def hash
@squares.hash
end

alias_method :==, :eql?

private

def non_empty_squares
@squares.reject(&:zero?)
end
end

class Row < Unit
def type
"Row"
end
end

class Column < Unit
def type
"Column"
end
end

class Box < Unit
def type
"Box"
end
end
end
59 changes: 59 additions & 0 deletions lib/sudoku_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
module Sudoku
class Validator
attr_reader :puzzle

def initialize(puzzle = nil)
raise(ArgumentError, "A Puzzle is required.") if puzzle.nil?
@puzzle = puzzle
end

def status
case [puzzle.valid?, puzzle.complete?]
when [true, true] then "This sudoku is valid."
when [true, false] then "This sudoku is valid, but incomplete."

else "This sudoku is invalid."
end
end

def errors
return [] if puzzle.valid?

[:rows, :columns, :boxes].map do |message|
puzzle.send(message).map do |index, unit|
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could save yourself a level of nesting here by using Puzzle#all_units.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, thanks @JoelQ. I will have to promote all_units from a private method to public though. Don't think this is too much of an issue...after all, rows, columns and boxes are all public and therefore all_units could be a convenience method to grab them all in one go. Will write a new spec for this as this will now be part of Puzzle's API.

duplicates(unit).map do |number, positions|
DuplicateError.new(unit.type, index, number, positions)
end
end
end.flatten
end

private

def duplicates(unit)
unit.positions.select do |number, position|
non_empty?(number) && duplicate?(position)
end
end

def non_empty?(number)
number != 0
end

def duplicate?(position)
position.is_a?(Array)
end
end

class DuplicateError < Struct.new(:type, :index, :number, :positions)
def to_s
"#{type} #{index} contains a duplicate #{number} in squares #{list(positions)}"
end

private

def list(positions)
"#{positions[0...-1].join(', ')} and #{positions.last}"
end
end
end
40 changes: 40 additions & 0 deletions spec/lib/sudoku_fileparser_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
require 'sudoku_fileparser'

module Sudoku
describe FileParser do
it "throws an ArgumentError if instantiated without a file" do
expect { FileParser.new }.to raise_error(ArgumentError)
end

it "can parse rows out of the file into a simple array format (strings are converted to numbers)" do
parser = FileParser.new(example_file)
parser.parse_rows.should eq [
[0, 5, 9, 6, 1, 2, 4, 3, 0],
[7, 0, 3, 8, 5, 4, 1, 0, 9],
[1, 6, 0, 3, 7, 9, 0, 2, 8],
[9, 8, 6, 0, 4, 0, 3, 5, 2],
[3, 7, 5, 2, 0, 8, 9, 1, 4],
[2, 4, 1, 0, 9, 0, 7, 8, 6],
[4, 3, 0, 9, 8, 1, 0, 7, 5],
[6, 0, 7, 4, 2, 5, 8, 0, 3],
[0, 9, 8, 7, 3, 6, 2, 4, 0]
]
end

def example_file
%Q{
. 5 9 |6 1 2 |4 3 .
7 . 3 |8 5 4 |1 . 9
1 6 . |3 7 9 |. 2 8
------+------+------
9 8 6 |. 4 . |3 5 2
3 7 5 |2 . 8 |9 1 4
2 4 1 |. 9 . |7 8 6
------+------+------
4 3 . |9 8 1 |. 7 5
6 . 7 |4 2 5 |8 . 3
. 9 8 |7 3 6 |2 4 .
}
end
end
end
Loading