refactoring ruby code inject issues

Question

Hi can someone explain to me why my inject isn't working here?

Am I using inject correctly here? For some reason my code gets caught up in an endless loop once this scenario comes into play (the 5th move usually in my game)

  def cpu_block_player
    winning_combinations = [[0,1,2],[3,4,5],[6,7,8],[0,3,6],[1,4,7],[2,5,8],[0,4,8],[2,4,6]]
    executed = 0
    winning_combinations.each do |combination|
      result_of_combination = ""
      result_of_combination = combination.inject("") {|result, element| result + @board[element]}
      if result_of_combination == "XXe" || result_of_combination == "eXX" || result_of_combination == "XeX"
        executed += 1
        puts executed 
        player_move(@current_turn, result_of_combination.index("e"))     
      end
      break if executed >= 1
   end

Answer 1

First of all, these kinds of questions are better suited for the Code Review Stack Exchange site.

However, here are my thoughts:

My first thought when looking at your code is that you're just having one large class. In order to see some real advantages with Object-Oriented Programming, I'd recommend extracting some of the code into separate classes. I can definitely see a Board class inside the Game class, just waiting to be extracted.

Some ideas for methods to add to a Board class:

• to_s -- This would be what's in your print_board method at the moment, without the print.
• finished? -- Check whether the game is "finished" (ie., someone has won). A winner method would also make sense.
• taken? -- Whether someone has taken a position before.

A lot of the code in your Game class would benefit from naming. For instance, take this piece of code:

@current_turn == @player_x ? @current_turn = @player_o : @current_turn = @player_x

It's not super hard to figure out what this piece of code does, but exactly how you swap who is the current player is probably not important to know while reading the player_move method. All you want to know is that "at this point, we switch players".

Extracting methods and objects doesn't make you write less code, but in my opinion it makes for clearer code. If you can give every piece of line a name (ie., extract it to a method), then it is probably a lot easier to figure out what is going on.