-
-
Notifications
You must be signed in to change notification settings - Fork 36
SudokuValidator submission #1
base: master
Are you sure you want to change the base?
Changes from all commits
19af7a1
e6974eb
741dee4
fbe316d
26eb987
72cb7a4
2bfb0d6
aa56284
fea34ae
b52f595
9ef05da
91dd600
6b45e1b
13f5728
3cc1a0f
2365921
5371e29
e5625b0
6d7bca3
6b0b03e
d9c66c1
dd8815e
209c60e
5abb52e
e51d13e
9207dd6
f4448ea
6a6957d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| *.sw[op] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| class SudokuBoard | ||
| attr_reader :board | ||
|
|
||
| def initialize(game_file) | ||
| @board = [] | ||
| File.readlines(game_file).each do |line| | ||
| row = convert_line_to_row(line) | ||
| @board << row unless row.nil? | ||
| end | ||
| end | ||
|
|
||
| def [] (row, col) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why name a method "[]" unless you have to? i think it will unnecessarily confuse callers
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, @elubin, this was for me a rather natural way to access a single element on the board and same for updating an element (used in the rspec examples) with the []= method. It's visually analogous to array accession. |
||
| @board[row-1][col-1] | ||
| end | ||
|
|
||
| def []= (row, col, value) | ||
| @board[row-1][col-1] = value | ||
| end | ||
|
|
||
| def row(row) | ||
| @board[row-1] | ||
| end | ||
|
|
||
| def col(col) | ||
| column = [] | ||
| @board.each { |row| column << row[col-1] } | ||
| column | ||
| end | ||
|
|
||
| def sub_grid(number) | ||
| # SubGrids are numbered 1-9 like this: | ||
| # 1,2,3 | ||
| # 4,5,6 | ||
| # 7,8.9 | ||
| row, col = sub_grid_start(number) | ||
| s_grid = [] | ||
| 3.times do | ||
| s_grid += sub_grid_row(row,col) | ||
| row += 1 | ||
| end | ||
| s_grid | ||
| end | ||
|
|
||
| def sub_grid_row(row,col) | ||
| [self[row,col],self[row,col+1],self[row,col+2]] | ||
| end | ||
|
|
||
| def convert_line_to_row(line) | ||
| line.chomp!.delete!('-+| ') | ||
| return nil if line.empty? | ||
| line.tr!('.','0') | ||
| row = [] | ||
| line.each_char.map { |c| row << c.to_i } | ||
| row | ||
| end | ||
|
|
||
| def sub_grid_start(sub_grid) | ||
| row = ((sub_grid-1)/3)*3 + 1 | ||
| col = ((sub_grid-1)%3)*3 + 1 | ||
| [row,col] | ||
| end | ||
|
|
||
| def missing_data? | ||
| !(self.board.flatten.index( 0 )).nil? | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| require_relative './sudoku_board' | ||
|
|
||
| class SudokuValidator | ||
| attr_reader :board, :errors | ||
|
|
||
| def initialize(game_file) | ||
| @game_file = game_file | ||
| @board = SudokuBoard.new @game_file | ||
| @errors = [] | ||
| end | ||
|
|
||
| def validate | ||
| status = check_game_status | ||
| validity = check_game_validity | ||
| [validity, status] | ||
| end | ||
|
|
||
| def check_game_status | ||
| @board.missing_data? ? 'incomplete' : 'complete' | ||
| end | ||
|
|
||
| def check_game_validity | ||
| rows_valid = check_row_validity | ||
| cols_valid = check_col_validity | ||
| subgrids_valid = check_subgrid_validity | ||
| (rows_valid && cols_valid && subgrids_valid) ? 'valid' : 'invalid' | ||
| end | ||
|
|
||
| def check_row_validity | ||
| valid_element?(:row) | ||
| end | ||
|
|
||
| def check_col_validity | ||
| valid_element?(:col) | ||
| end | ||
|
|
||
| def check_subgrid_validity | ||
| valid_element?(:sub_grid) | ||
| end | ||
|
|
||
| def valid_element?(element) | ||
| item_valid = true | ||
| (1..9).each do |item| | ||
| valid, error = valid?(@board.__send__(element,item)) | ||
| item_valid &&= valid | ||
| report_errors(error, element, item) | ||
| end | ||
| item_valid | ||
| end | ||
|
|
||
| def valid?(ary) | ||
| tmp = ary.map { |e| e if e !=0}.compact | ||
| valid = tmp.uniq.size == tmp.size | ||
| error = [] | ||
| error = identify_errors(tmp) if !valid | ||
| [valid,error] | ||
| end | ||
|
|
||
| def identify_errors (ary) | ||
| tmp = ary.clone | ||
| error = [] | ||
| while tmp.size > 0 do | ||
| item = tmp.shift | ||
| error << item if tmp.include? item | ||
| end | ||
| error | ||
| end | ||
|
|
||
| def report_errors (ary,where,item) | ||
| ary.each { |e| @errors << "#{e} is repeated in #{where} #{item}" } | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| require_relative '../lib/sudoku_board' | ||
|
|
||
| describe SudokuBoard do | ||
| before do | ||
| @game_vc = SudokuBoard.new("valid_complete.sudoku") | ||
| @game_ii = SudokuBoard.new("invalid_incomplete.sudoku") | ||
| end | ||
|
|
||
| describe "#[]" do | ||
| it "returns the first character of the sudoku file" do | ||
| expect(@game_vc[1,1]).to eql 8 | ||
| end | ||
|
|
||
| it "returns an arbitrary element from the board" do | ||
| expect(@game_vc[4,5]).to eql 4 | ||
| end | ||
|
|
||
| it "returns zero for an empty position" do | ||
| expect(@game_ii[2,4]).to eql 0 | ||
| end | ||
| end | ||
|
|
||
| describe "#row" do | ||
| it "returns the first row of the sudoku file" do | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about creating own
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @mrhead... I wasn't sure at first if I thought this was a good idea: I like the idea of a describe block to separate complete/incomplete board examples - that's the way I started my development. However, after some reflection I think your suggested refactoring makes sense... while not immediately necessary, in the long run it will more clearly isolate the individual elements being tested. I'll work on that fairly soon.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK; done. The only thing I'm not sure I like about this is the before block which now loads the valid_complete file and the invalid_incomplete file to support the different tests, creating @game_vc & @game_ii instances. I don't think that creating additional describe blocks makes sense -- lots more code and repeated before blocks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've stopped using From beginning I did not know how to replace it, but now I create my own methods to prepare objects for me. So unlike in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see what you mean. I'd like to see some explanation as to why some if these are considered the way to do things... Sent from my iPhone
|
||
| expect(@game_vc.row(1)).to eql [8,5,9,6,1,2,4,3,7] | ||
| end | ||
|
|
||
| it "returns the expected row" do | ||
| expect(@game_ii.row(4)).to eql [0,0,0,1,0,7,0,0,2] | ||
| end | ||
| end | ||
|
|
||
| describe "#col" do | ||
| it "returns the first column of the sudoku file" do | ||
| expect(@game_vc.col(1)).to eql [8,7,1,9,3,2,4,6,5] | ||
| end | ||
|
|
||
| it "returns the expected column" do | ||
| expect(@game_ii.col(8)).to eql [0,0,0,0,0,0,7,0,4] | ||
| end | ||
| end | ||
|
|
||
| describe "#sub_grid" do | ||
| it "returns a 3x3 sub-grid as an array" do | ||
| expect(@game_vc.sub_grid(1)).to eql [8,5,9,7,2,3,1,6,4] | ||
| end | ||
|
|
||
| it "returns a middle 3x3 sub-grid" do | ||
| expect(@game_vc.sub_grid(5)).to eql [1,4,7,2,6,8,5,9,3] | ||
| end | ||
|
|
||
| it "successfully returns the last sub-grid" do | ||
| expect(@game_vc.sub_grid(9)).to eql [6,7,5,8,9,3,2,4,1] | ||
| end | ||
|
|
||
| it "successfully returns the first sub-grid" do | ||
| expect(@game_vc.sub_grid(1)).to eql [8,5,9,7,2,3,1,6,4] | ||
| end | ||
|
|
||
| it "returns the expected sub-grid" do | ||
| expect(@game_ii.sub_grid(8)).to eql [0,8,0,0,0,0,0,3,6] | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| require_relative '../lib/sudoku_validator' | ||
| require_relative '../lib/sudoku_board' | ||
|
|
||
| describe SudokuValidator do | ||
|
|
||
| describe "valid games" do | ||
| it "correctly recognizes a valid, complete game" do | ||
| result = %x{"./sudoku_validator" "./valid_complete.sudoku"} | ||
| expect(result).to eq "This sudoku is valid, and complete.\n" | ||
| end | ||
|
|
||
| it "recognizes a valid, incomplete game" do | ||
| result = `./sudoku_validator ./valid_incomplete.sudoku` | ||
| expect(result).to eq "This sudoku is valid, and incomplete.\n" | ||
| end | ||
| end | ||
|
|
||
| describe "invalid games" do | ||
| it "recognizes an invalid, complete game" do | ||
| result = `./sudoku_validator ./invalid_complete.sudoku` | ||
| expect(result).to include "This sudoku is invalid, and complete.\n" | ||
| expect(result).to include "2 is repeated in col 6" | ||
| end | ||
| end | ||
|
|
||
| describe "error handling" do | ||
| before do | ||
| @game = SudokuValidator.new "./invalid_complete.sudoku" | ||
| @element = [1,2,3,2,5,6,7,8,9] | ||
| end | ||
|
|
||
| it "identifies correct element in #identify_errors" do | ||
| expect(@game.identify_errors @element).to eql [2] | ||
| end | ||
|
|
||
| it "returns errors in #valid?" do | ||
| expect(@game.valid?(@element)).to eq [false, [2]] | ||
| end | ||
|
|
||
| it "returns empty error array in #vaid? if no errors" do | ||
| @element[3]=4 | ||
| expect(@game.valid?(@element)).to eq [true, []] | ||
| end | ||
|
|
||
| it "returns the descriptive error string for col errors" do | ||
| @game.validate | ||
| @game.errors.should include "2 is repeated in col 6" | ||
| end | ||
|
|
||
| it "returns the descriptive error string for row errors" do | ||
| @game.board[1,1] = 7 | ||
| @game.validate | ||
| @game.errors.should include "7 is repeated in row 1" | ||
| end | ||
|
|
||
| it "returns the descrptive error string for sub-grid errors" do | ||
| @game.board[1,1] = 7 | ||
| @game.validate | ||
| @game.errors.should include "7 is repeated in sub_grid 1" | ||
| end | ||
|
|
||
| it "ignores missing (zero) elements for incomplete sudokus" do | ||
| @game.board[1,1] = 0 | ||
| @game.board[1,7] = 0 | ||
| @game.board[1,4] = 0 | ||
| @game.validate | ||
| @game.errors.should_not include "0 is repeated in row 1" | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #!/usr/bin/env ruby | ||
| require_relative 'lib/sudoku_validator' | ||
|
|
||
| @validator = SudokuValidator.new(ARGV[0]) | ||
| validity, status = @validator.validate | ||
| puts "This sudoku is #{validity}, and #{status}." | ||
| @validator.errors.each { |err| puts err } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this file is, but it's possible to configure a global gitignore so that you don't need to do this for each repo you work on: https://help.github.com/articles/ignoring-files#global-gitignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @andyw8; what's very interesting is that I already have a ~/.gitignore_global file and *.swp is there and a .swp file still, somehow got included. I'll have to do some digging to see why that is. Oh, and they're vim swap files.