ansaurus

Question

how to make my first Ruby effort more idiomatic

Answer 1

+2 A:

Here's a quick solution to how I would do this. There may be a lot to digest in here, so feel free to ask questions, but reading the Ruby Array or Enumerable documentation should answer most of them about some of things I did, and there's tons of good tutorials on ruby classes. Here's a good one for understanding the accessors I used in the class here instead of the struct.

class Game
  attr_accessor :header, :moves
  def initialize
    self.header = []
  end
end

games = []
game = Game.new
File.open('jemptymethod.pgn').each_line do |line|
  next if line.chomp.empty?
  if game.moves
    games << game
    game = Game.new
  end
  if /^\[/.match(line)
    game.header << line
  else
    moves = line.split(/\d+\.\s*/) # splitting on the move numbers so that we don't have to iterate through to remove them
    moves.shift # getting rid of first empty move since the split on '1. ' created an array element before the '1. '
    game.moves = moves
  end
end
games << game # add last game since the first part of the file loop doesn't execute again to do it

puts "# Games: " + games.length.to_s

first_moves = games.map {|game| game.moves[0]} # Could easily iterate over the size of the longest game to get other moves (eg second move, etc)
first_moves_count = first_moves.inject(Hash.new(0)) {|h, move| h[move] += 1; h} # Read ruby documentation on inject to see how this works
first_moves_count.each do |move, count|
  puts "1. #{move} occurred #{count} times"
end

mmrobins 2009-12-19 18:00:44

Answer 2

+1 A:

I haven't done a complete refactoring because I want to keep enough of your original code intact that it's not too confusing. The major change is introduction of a Game class that handles parsing. The implementation of this class can be much improved, but it works without changing your code too much. Also, some minor points:

Instead of File.new, read the file using File.open and give it a block that takes the file parameter. The file is automatically closed at the end of the block.
Use of a += 1 instead of a = a + 1.

I made up a simple notation and wrote a parser for handling the play-by-play details of a tennis match. You might want to look at that code for an example of parsing game moves. It's actually very similar to what you're doing. The bulk of the code is in the /lib directory. Parsing logic is in parser.rb and game components are in the other files. I'd encourage you to break your chess games up in a similar way, by adding a Move class.

Anyway, here's my half-refactoring of your code:

class Game
  attr_accessor :header, :score, :moves

  def initialize
    @header = ""
    @score  = ""
    @moves  = []
  end

  def first_move
    moves_index.index(moves[0])
  end

  def moves_index
    moves_index = {}
    score.split(/\s+/).each_with_index do |move,i|
      if i%3 != 0
        unless moves_index.has_key?(move)
          moves_index[move] = moves_index.keys.length
        end
        moves << moves_index[move]
      end
    end
    moves_index
  end
end

games     = []
is_header = false
is_score  = false

File.open("jemptymethod.pgn") do |file|
  while (line = file.gets)
    if !line.chomp.empty?
      if !is_score && !is_header
        game = Game.new
      end
      if line[0,1] == '['
        is_header = true
        game.header << line
      else
        is_score = true
        game.score << line
      end
    elsif is_score
      is_score = false
      is_header = false
      games << game
    end
  end
end

puts "# Games: " + games.length.to_s
first_moves = {}

#the following output should essentially be lossless
#with the possible exception of beginning or ending newlines
#
#puts gm.header + "\n"
#puts gm.score + "\n"
games.each do |gm|
  if !first_moves.has_key?(gm.first_move)
    first_moves[gm.first_move] = 1
  else
    first_moves[gm.first_move] += 1
  end
end

# sorting hashes by value: http://nhw.pl/wp/2007/06/11/sorting-hash-by-values
first_moves.sort{|a,b| -1*(a[1]<=>b[1])}.each{|k,v|
  puts "1. #{k} occurred #{v} times" 
}

Alex Reisner 2009-12-19 18:52:10

I accept this one because it produces the same output as my original program.

George Jempty 2009-12-21 12:13:26

ansaurus

tags:

views:

answers:

how to make my first Ruby effort more idiomatic

related questions