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
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
2 changes: 2 additions & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--color
--format Fuubar
9 changes: 9 additions & 0 deletions bin/sudoku-validator
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env ruby

require './lib/sudoku_reader'
require './lib/sudoku_validator'

reader = SudokuReader.new(ARGV.first)
validator = SudokuValidator.new(reader)

puts validator.message
11 changes: 11 additions & 0 deletions invalid_block.sudoku
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
8 5 9 |6 1 2 |4 3 7
7 2 3 |8 5 4 |1 6 9
1 6 4 |3 7 9 |5 2 8
------+------+------
9 8 6 |1 4 7 |3 5 2
3 7 . |2 6 8 |9 1 4
2 4 1 |5 9 3 |7 8 6
------+------+------
4 3 5 |9 8 1 |6 7 .
6 1 7 |4 2 5 |8 9 3
5 9 8 |7 3 6 |2 4 1
11 changes: 11 additions & 0 deletions invalid_column.sudoku
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
8 5 9 |6 1 2 |4 3 7
7 2 3 |8 5 4 |1 6 9
1 6 4 |3 7 9 |5 2 8
------+------+------
9 8 6 |1 4 7 |3 5 2
3 7 5 |8 6 2 |9 1 4
2 4 1 |5 9 3 |7 8 6
------+------+------
4 3 2 |9 8 1 |6 7 5
6 1 7 |4 2 5 |8 9 3
5 9 8 |7 3 6 |2 4 1
11 changes: 11 additions & 0 deletions invalid_line.sudoku
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
8 5 9 |6 5 2 |4 3 7
7 2 3 |8 1 4 |1 6 9
1 6 4 |3 7 9 |5 2 8
------+------+------
9 8 6 |1 4 7 |3 5 2
3 7 5 |2 6 8 |9 1 4
2 4 1 |5 9 3 |7 8 6
------+------+------
4 3 2 |9 8 1 |6 7 5
6 1 7 |4 2 5 |8 9 3
5 9 8 |7 3 6 |2 4 1
91 changes: 91 additions & 0 deletions lib/sudoku_reader.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
class SudokuReader
attr_reader :path

def initialize(path)
@path = path
@lines = []
@blocks = []
end

def lines
return @lines if @lines.any?
File.read(path).each_line {|line| convert_line_to_values(line)}
@lines
end

def each_line
lines.each_with_index {|line, index| yield(line, index)}
end

def each_value_by_line
each_line {|line, index| line.each{|value| yield(line, value, index)}}
end

def each_value_by_column
columns.each_with_index do |column, index|
column.each{|value| yield(column, value, index)}
end
end

def each_value_by_block
blocks.each_with_index do |block, index|
block.each{|value| yield(block, value, index)}
end
end

def columns
@columns ||= lines.transpose
Copy link

Choose a reason for hiding this comment

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

Wow... This is the reason why I love ruby; I can't believe transpose is a standard lib method

end

def blocks
Copy link

Choose a reason for hiding this comment

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

i find this method hard to read. i would prefer if it just initialized blocks and you filled it somewhere else. something like:
@blocks ||= 9.times { @blocks << [] }

no reason to keep the global @blocks when you can call this helper

return @blocks if @blocks.any?
init_empty_blocks
fill_blocks
@blocks
end

private
def fill_blocks
Copy link

Choose a reason for hiding this comment

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

this method gets a little complicated. i would prefer to see better variable names (i,j,k) to understand what's going on

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I don't like this method too. I will check for another solution.

each_block do |block_number|
line, column = block_start(block_number)
each_line_in_block {|k| get_line_of_block(line, column, k)}
end
end

def block_start(block_number)
row = ((block_number) / 3) * 3
col = ((block_number) % 3) * 3
[row,col]
end

def get_line_of_block(vertical_start, horizontal_start, line_block)
line = lines[horizontal_start + line_block]
line_in_block = line.drop(vertical_start).first(3)
index_of_block = vertical_start + horizontal_start/3
@blocks[index_of_block] += line_in_block
end

def each_block
9.times {|block_number| yield(block_number)}
end

def each_line_in_block
3.times {|index_line_of_block| yield(index_line_of_block)}
end

def init_empty_blocks
9.times { @blocks << [] }
end

def convert_line_to_values(line)
@lines << line.split.map { |value| sanitize(value) } unless separator?(line)
end

def separator?(line)
line == "------+------+------\n"
end

def sanitize(value)
value.gsub('|','').to_i
end
end
51 changes: 51 additions & 0 deletions lib/sudoku_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
class SudokuValidator
attr_accessor :reader

def initialize(reader)
@reader = reader
@errors = []
end

def complete?
reader.each_line {|line| return false if line.include?(0)}
return true
end

def valid?
lines_valid? && columns_valid? && blocks_valid?
Copy link

Choose a reason for hiding this comment

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

I think this might be larger design question, but running validation here seems to be tightly coupled with the *valid? method. Do you think this is a good design? Should you separate valid? checking and error generation?

I did the exact same thing at first, but later decided to take the approach of a separate #validate! method.

Copy link
Author

Choose a reason for hiding this comment

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

You're probably right. The responsability of this class is to check if the sudoku is valid or not. Build the message is another responsability. Check if the sudoku is complete can be another responsability (so another class) too.

In ActiveRecord, the is the same problem. If you do something like user.valid? it fill user.errors with messages.

For this project, I will not change the code because it will not grow. So, yes, it's better but no, I would not do it.

end

def message
if valid?
"This sudoku is valid#{complete_message}."
else
"This sudoku is invalid." + @errors.join
end
end

private
def complete_message
", but incomplete" unless complete?
end

def lines_valid?
check_validity_for(:line)
end

def columns_valid?
check_validity_for(:column)
end

def blocks_valid?
check_validity_for(:block)
end

def check_validity_for(type)
reader.send("each_value_by_#{type}") do |row, value, index|
if row.count(value) > 1 && value != 0
@errors << "\n - There is multiple #{value} in #{type} #{index + 1}."
end
end
@errors.empty?
end
end
39 changes: 39 additions & 0 deletions spec/sudoku_reader_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
require_relative '../lib/sudoku_reader'

describe SudokuReader do
describe "#lines" do
it "give lines" do
reader = SudokuReader.new('valid_complete.sudoku')
expect(reader.lines[0]).to eq [8,5,9,6,1,2,4,3,7]
end

it "convert invalid values by 0" do
reader = SudokuReader.new('valid_incomplete.sudoku')
expect(reader.lines[0]).to eq [8,5,0,0,0,2,4,0,0]
end
end

describe "#columns" do
it "give columns" do
reader = SudokuReader.new('valid_complete.sudoku')
expect(reader.columns[0]).to eq [8,7,1,9,3,2,4,6,5]
end

it "convert invalid values by 0" do
reader = SudokuReader.new('valid_incomplete.sudoku')
expect(reader.columns[0]).to eq [8,7,0,0,3,0,0,0,0]
end
end

describe "#blocks" do
it "give blocks" do
reader = SudokuReader.new('valid_complete.sudoku')
expect(reader.blocks[0]).to eq [8,5,9,7,2,3,1,6,4]
end

it "convert invalid values by 0" do
reader = SudokuReader.new('valid_incomplete.sudoku')
expect(reader.blocks[0]).to eq [8,5,0,7,2,0,0,0,4]
end
end
end
81 changes: 81 additions & 0 deletions spec/sudoku_validator_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
require_relative '../lib/sudoku_validator'
require_relative '../lib/sudoku_reader.rb'

describe SudokuValidator do
describe "#complete?" do
context "with complete sudokus" do
it "give true" do
['valid_complete.sudoku', 'invalid_complete.sudoku'].each do |file|
expect(validator_for(file)).to be_complete
end
end
end

context "with incomplete sudokus" do
it "give false" do
['valid_incomplete.sudoku', 'invalid_incomplete.sudoku'].each do |file|
expect(validator_for(file)).to_not be_complete
end
end
end
end

describe "#valid?" do
context "with valid sudokus" do
it "give true" do
['valid_complete.sudoku', 'valid_incomplete.sudoku'].each do |file|
expect(validator_for(file)).to be_valid
end
end
end

context "with invalid sudokus" do
it "give true" do
['invalid_complete.sudoku',
'invalid_incomplete.sudoku',
'invalid_column.sudoku',
'invalid_block.sudoku',
'invalid_line.sudoku'].each do |file|
expect(validator_for(file)).to_not be_valid
end
end
end
end

describe "#message" do
context "when the sudoku is valid" do
context "when the sudoku is complete" do
it "say than the sudoku is valid" do
file = 'valid_complete.sudoku'
expect(validator_for(file).message).to eq "This sudoku is valid."
end
end

context "when the sudoku is incomplete" do
it "say than the sudoku is valid" do
file = 'valid_incomplete.sudoku'
expect(validator_for(file).message).to eq "This sudoku is valid, but incomplete."
end
end
end

context "when the sudoku is invalid" do
it "say than the sudoku is invalid" do
['invalid_complete.sudoku', 'invalid_incomplete.sudoku'].each do |file|
expect(validator_for(file).message).to match "This sudoku is invalid."
end
end

context "when the sudoku is invalid du to lines" do
it "say than there is an error in line" do
file = 'invalid_line.sudoku'
expect(validator_for(file).message).to match "There is multiple 5 in line 1."
end
end
end
end

def validator_for(file)
SudokuValidator.new(SudokuReader.new(file))
end
end
18 changes: 9 additions & 9 deletions valid_complete.sudoku
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
8 5 9 |6 1 2 |4 3 7
7 2 3 |8 5 4 |1 6 9
1 6 4 |3 7 9 |5 2 8
8 5 9 |6 1 2 |4 3 7
7 2 3 |8 5 4 |1 6 9
1 6 4 |3 7 9 |5 2 8
------+------+------
9 8 6 |1 4 7 |3 5 2
3 7 5 |2 6 8 |9 1 4
2 4 1 |5 9 3 |7 8 6
9 8 6 |1 4 7 |3 5 2
3 7 5 |2 6 8 |9 1 4
2 4 1 |5 9 3 |7 8 6
------+------+------
4 3 2 |9 8 1 |6 7 5
6 1 7 |4 2 5 |8 9 3
5 9 8 |7 3 6 |2 4 1
4 3 2 |9 8 1 |6 7 5
6 1 7 |4 2 5 |8 9 3
5 9 8 |7 3 6 |2 4 1