← Back to Upcase

Help Me Refactor A Tic Tac Toe Tokenizer


(Nick R) #1

I built a little Tic Tac Toe Tokenizer in ruby and I’d love to have some discussion around it.

require 'strscan'
require 'stringio'

class Token
  attr_reader :kind, :value
  def initialize(kind, value)
    @kind = kind
    @value = value
  end

  def to_s
    "#{kind}:\t#{value}"
  end
end

class Tokenizer
  attr_reader :io
  XTOKEN = /X/
  OTOKEN = /O/
  BLANKTOKEN = /-/

  def initialize(io)
    @io = StringScanner.new io
  end

  def next_token
    return nil if io.eos?

    if token = io.scan(XTOKEN)
      return Token.new(:x, token)
    elsif token = io.scan(OTOKEN)
      return Token.new(:y, token)
    elsif token = io.scan(BLANKTOKEN)
      return Token.new(:blank, token)
    else
      # Unknown token
    end
  end
end

tokenizer = Tokenizer.new "XX-O--O--"
while (token = tokenizer.next_token)
  puts token
end

# => x:	X
# => x:	X
# => blank:	-
# => y:	O
# => blank:	-
# => blank:	-
# => y:	O
# => blank:	-
# => blank:	-

On nil

nil usually isn’t a good value to return from a function- it’s devoid of meaning. I’ve enjoyed some of the early videos on nil in the upcase library. But here, in the next_token method I’m returning nil when there are no more tokens. I can make an argument that because there are no more token that this might be a value- you can keep calling next_token as much you like and you’ll always get nil. But I bet there’s something that would feel better.

What should I be returning? Should I raise a NoMoreTokens exception at that point? Is that pushing complexity up the stack? Id love your thoughts.

OO Design

I have a token class here that stores a kind instance variables. That feels bad right away, right? I bet the klaxons of Object Oriented design are going off in your mind. That should be a class! (right?) Consider:

class Token
  attr_reader :value
  def initialize(value)
    @value = value
  end
end

class XToken < Token; end
class YToken < Token; end
class BlankToken < Token; end

Is that better? Why or why not?

What do you think of a class ending in erTokenizer?

Unknown token

Right now nothing happens when an unknown token is encountered. Is that an exception? Should it raise an UnknownTokenException?

Other thoughts?

Is there anything else that jumps out about this code? That’s good or bad?

Thanks for a taking a look!