Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

It looks like it should be shortened but I can't see how.

I have my methods <= 5 lines but that's as far as I got:

def check_lines
  if horizontal?(@player) || vertical?(@player) || diagonal?(@player)
    @win=@player
  end
end

def horizontal?(player)
  ((@squares[0] == player) && (@squares[1] == player) && (@squares[2] == player)) ||
  ((@squares[3] == player) && (@squares[4] == player) && (@squares[5] == player)) ||
  ((@squares[6] == player) && (@squares[7] == player) && (@squares[8] == player))
end

def vertical?(player)
  ((@squares[0] == player) && (@squares[3] == player) && (@squares[6] == player)) ||
  ((@squares[1] == player) && (@squares[4] == player) && (@squares[7] == player)) ||
  ((@squares[2] == player) && (@squares[5] == player) && (@squares[8] == player))
end

def diagonal?(player)
  ((@squares[0] == player) && (@squares[4] == player) && (@squares[8] == player)) ||
  ((@squares[2] == player) && (@squares[4] == player) && (@squares[6] == player))
end
share|improve this question

2 Answers 2

up vote 11 down vote accepted

When all of your code is the same, just with different numbers, you want to use data-directed programming.

WINS = [
  [0, 1, 2], [3, 4, 5], [6, 7, 8],  # <-- Horizontal wins
  [0, 3, 6], [1, 4, 7], [2, 5, 8],  # <-- Vertical wins
  [0, 4, 8], [2, 4, 6],             # <-- Diagonal wins
]

def check_lines
  if WINS.any? { |line| line.all? { |square| @squares[square] == @player } }
    @win = @player
  end
end
share|improve this answer
    
Nice, clean, easy-to-read solution. –  Cary Swoveland May 4 '14 at 17:52

[Edit: a couple of days ago an anonymous user proposed an edit that was rejected by two members, both on the grounds that "the edit deviates from the original intent of the post." While I'm sure those members had good intentions, neither appears to have experience with Ruby, so I don't understand how they were able to arrive at that conclusion.

The anonymous user was not just improving my code, he or she was correcting an error I made. It was a good catch and a good fix. I've edited to incorporate the fix (and made a few other small changes).

If you are that anonymous user, please accept my thanks. A small suggestion: it probably would have been better if you had just left a comment, but the main thing is that the code is now fixed. (I hope.)]


You can also use Ruby's Matrix and Vector classes to see if player p wins. In the following, square[i][j] is the player in row i, column j.

Code

require 'matrix'

def win?(square,p)
  n = square.size
  m = Matrix[*square]
  pvec = Matrix.build(1,n){p}.row(0)
  m.row_vectors.any?    { |r| r == pvec }      ||
    m.column_vectors.any? { |c| c == pvec }    ||
    Vector[*m.each(:diagonal).to_a] == pvec    ||
    n.times.all? { |i| square[i][n-i-1] == p }
end

Example

square = [[1,3,2],
          [4,2,6],
          [2,8,9]]

win?(square, 2)                    #=> true
win?([[1,3,2],[4,3,6],[2,3,9]], 3) #=> true
win?([[1,3,2],[3,3,3],[2,7,9]], 3) #=> true
win?([[1,3,2],[4,5,6],[7,8,9]], 1) #=> false

Explanation

pvec = Matrix.build(1,n){p}.row(0)

creates a vector for which every element is the value of the player argument p (e.g., pvec # => Vector[2,2,2]).

m.row_vectors.any? { |r| r == pvec }

determines if player p wins in any row,

m.column_vectors.any? { |c| c == pvec }

determines if player p wins in any column,

Vector[*m.each(:diagonal).to_a] == pvec

determines if player p wins on the main diagonal, and

(0...n).all? { |i| square[i][n-i-1] == p }

determines if player p wins on the minor diagonal (top right to bottom left).

I was unable to find a way to check the minor diagonal using Matrix class methods.

share|improve this answer
    
I wouldn't advise the OP to use this for a simple tic-tac-toe game, but it's interesting to show the capabilities of Matrix nonetheless. –  tokland May 6 '14 at 7:03
    
@MannyMeng: a couple of days ago you rejected an edit proposed by "an anonymous user". If you have access to that user's SO name, please let me know, as I'd like to thank him or her. –  Cary Swoveland Jun 8 at 4:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.