Pig Latin Code Kata

Question

I've just complete a Pig Latin translator as a code kata and I was hoping someone would review it for me.

Here is the kata:

PigLatin Kata
   Create a PigLatin class that is initialized with a string
      - detail: The string is a list of words separated by spaces: 'hello world'
      - detail: The string is accessed by a method named phrase
      - detail: The string can be reset at any time without re-initializing
      - example: PigLatin.new('hello world')

completed (Y|n):

   Translate Method
      Create a translate method that translates the phrase from english to pig-latin.
         - detail: The method will return a string.
         - detail: The empty string will return nil.
         - example: "" translates to nil

completed (Y|n):

      Translate words that start with vowels.
         - detail: Append "ay" to the word if it ends in a consonant.
         - example: "ask" translates to "askay"
         - detail: Append "yay" to the word if it ends with a vowel.
         - example: "apple" translates to "appleyay"
         - detail: Append "nay" to the word if it ends with "y".
         - example: "any" translates to "anynay"

completed (Y|n):

      Translate a word that starts with a single consonant.
         - detail: Removing the consonant from the front of the word.
         - detail: Add the consonant to the end of the word.
         - detail: Append 'ay' to the resulting word.
         - example: "hello" translates to "ellohay"
         - example: "world" translates to "orldway"

completed (Y|n):

      Translate words that start with multiple consonants.
         - detail: Remove all leading consonants from the front of the word.
         - detail: Add the consonants to the end of the word.
         - detail: Append 'ay' to the resulting word.
         - example: "known" translates to "ownknay"
         - example: "special" translates to "ecialspay"

completed (Y|n):

      Support any number of words in the phrase.
         - example: "hello world" translates to "ellohay orldway"
         - detail: Each component of a hyphenated word is translated seperately.
         - example: "well-being" translates to "ellway-eingbay"

completed (Y|n):

      Support capital letters.
         - detail: If a word is captalized, the translated word should be capitalized.
         - example: "Bill" translates to "Illbay"
         - example: "Andrew" translates to "Andreway"

completed (Y|n):

      Retain punctuation.
         - detail: Punctuation marks should be retained from the source to the translated string
         - example: "fantastic!" tranlates to "anfasticfay!"
         - example: "Three things: one, two, three." translates to "Eethray ingsthay: oneyay, otway, eethray."

completed (Y|n):



Congratulations!
- Create a PigLatin class that is initialized with a string                00:12:52
- Create a translate method that translates the phrase from english to p   00:03:00
- Translate words that start with vowels.                                  00:08:56
- Translate a word that starts with a single consonant.                    00:04:32
- Translate words that start with multiple consonants.                     00:08:08
- Support any number of words in the phrase.                               00:25:19
- Support capital letters.                                                 00:05:05
- Retain punctuation.                                                      00:17:00
----------------------------------------------------------------------
Total Time taking PigLatin kata:                                           01:24:52

And my specs:

require 'spec_helper'
require 'piglatin'

describe PigLatin do
  let(:pig_latin) { PigLatin.new(string) }
  let(:string) { "hello world" }

  describe "new" do
    specify { expect { pig_latin }.to_not raise_error }
  end

  describe ".phrase" do
    subject { pig_latin.phrase }
    it { should eq("hello world") }
    it "can reset the phrase" do
      pig_latin
      pig_latin.phrase = "world hello"
      subject.should eq("world hello")
    end
  end

  describe ".translate" do
    subject { pig_latin.translate }
    its(:class) { should eq(String) }

    context "empty string" do
      let(:string) { "" }
      it { should eq(nil) }
    end

    context "words that start with vowels" do
      let(:string) { "ask" }
      it { should eq("askay") }

      context "and also ends with a vowel" do
        let(:string) { "apple" }
        it { should eq("appleyay") }
      end

      context "and ends with y" do
        let(:string) { "any" }
        it { should eq("anynay") }
      end
    end

    context "words that start with a single consonant" do
      let(:string) { "hello" }
      it { should eq("ellohay") }
    end

    context "words that start with multiple consonants" do
      context "known" do
        let(:string) { "known" }
        it { should eq("ownknay") }
      end
      context "special" do
        let(:string) { "special" }
        it { should eq("ecialspay") }
      end
    end

    context "multiple words" do
      let(:string) { "hello world" }
      it { should eq("ellohay orldway") }

      context "hyphenated words" do
        let(:string) { "well-being" }
        it { should eq("ellway-eingbay") }
      end
    end

    context "Capitalization" do
      context "Bill" do
        let(:string) { "Bill" }
        it { should eq("Illbay") }
      end
      context "Andrew" do
        let(:string) { "Andrew" }
        it { should eq("Andreway") }
      end
    end

    context "Retain Punctuation" do
      context "fantastic!" do
        let(:string) { "fantastic!" }
        it { should eq("antasticfay!") }
      end
      context "Three things: one, two, three." do
        let(:string) { "Three things: one, two, three." }
        it { should eq("Eethray ingsthay: oneyay, otway, eethray.") }
      end
    end
  end
end

And, finally, the PigLatin class:

class PigLatin
  attr_accessor :phrase

  def phrase=(string)
    @phrase = string
  end

  alias :initialize :phrase=

  def translate
    return nil if (@phrase.empty?)

    word_array = @phrase.split(/\s/)
    word_array.collect! do |word|
      translate_word(word)
    end
    translated_phrase = word_array.join(" ")

    return translated_phrase
  end

  private

  def translate_word word
    words = word.split("-")
    words.collect! do |word|
      punctuation = word.slice!(/\W/)

      if (word[0] =~ /[aeiou]/i)
        case word[-1]
        when /[aeiou]/
          word += "yay"
        when /y/
          word += "nay"
        else
          word += "ay"
        end
      else
        consonants = word.slice!(/^[^aeiou]*/)
        word.capitalize! if (consonants.downcase!)
        word += consonants + "ay"
      end

      if (punctuation)
        word << punctuation
      end

      word
    end
    return words.join("-")
  end
end

Answer 1

Good marks for:

• Formatting
• Naming
• Each spec tests just one thing
• Coverage of edge cases such as empty string.

I think I would change the name from PigLatin to PigLatinTranslator. I can't think of a "pig latin" as a thing.

Was the API required by the challenge? If not, I'd consider simplifying it. There's not much reason for the translator to have any state. You can simplify it if the #translate method takes the string to be translated. You won't need the accessors or the #initialize method.

That #translate returns nil if the phrase is an empty string is a little surprising. I expect it to return an empty string.

Instead of splitting on white space, consider using gsub to scan for words and replace them. If you do that, you can remove all of #translate_word's special handling of punctuation:

  def translate
    return nil if @phrase.empty?
    @phrase.gsub(/\w+/) do |word|
      translate_word(word)
    end
  end

return word at the end of #translate_word can be simply word. The only reason to use the return keyword is for a return before the end of the method.

The sequence aeiou might deserve to be a constant:

VOWEL = 'aeiou'

  ...

  if word[0] =~ /[#{VOWEL}]/i
  ...
    when /[#{VOWEL}]/
  ...
    consonants = word.slice!(/[^#{VOWEL}]*/)

Handling of capitalization can be handled by, at the beginning of the function, noticing whether or not the word is capitalized, and then downcasing it:

is_capitalized = word =~ /^A-Z/
word = word.downcase

and at the end, capitalizing it again if needed.

word = word.capitalized if is_capitalized

This removes considerations of case from the rest of the function.

Expressions like word[0] =~ /.../ can be replaced with word =~ /^.../. Similarly, word[1] =~ /.../ can be replaced with `word =~ /...$/

Taken altogether, these suggestions yield a #translate_word more like this:

  def translate_word(word)
    is_capitalized = word =~ /^[A-Z]/
    word = word.downcase
    if (word =~ /^#{VOWEL}/i)
      case word
      when /#{VOWEL}$/
        word += "yay"
      when /y$/
        word += "nay"
      else
        word += "ay"
      end
    else
      consonants = word.slice!(/^[^#{VOWEL}]*/)
      word += consonants + "ay"
    end
    word = word.capitalize if is_capitalized
    word
  end

Answer 2

Expanding on part of Wayne's post, but taking a more declarative approach which avoids the case statement and makes the high level logic more visible, I came up with this:

VOWEL = "[aeiou]"
def translate_word(word)
  restore_capitalization = (word =~ /^[A-Z]/) ? :capitalize : :to_s
  ret = word.downcase

  ret = (ret =~ /^#{VOWEL}/i) ? vowel_translator(ret) : consonant_translator(ret)
  ret.send(restore_capitalization)
end

def vowel_translator(word)
  suffixes = [ {re: /#{VOWEL}$/, suffix: 'yay'}, {re: /y$/, suffix: 'nay'}, {re: /./, suffix: 'ay'} ]
  correct_suffix = suffixes.find {|h| word =~ h[:re]}[:suffix]
  word + correct_suffix
end

def consonant_translator(word)
  consonants = word.slice!(/^[^#{VOWEL}]*/)
  word + consonants + "ay"
end

Answer 3

Here is another way to write the translate_word method. I decided to subclass String, thinking it might improve readability.

class Vowel < String; end
class Consonant < String; end
class Y < Consonant; end

YAY = %w[y a y]
NAY = %w[n a y]
AY  = %w[a y]

def assign_chars_to_classes(word)
  raise ArgumentError, "Choked on #{word}" unless word =~ /^[a-z]*$/i
  word.downcase.chars.each_with_object([]) do |c,a|
    a << if "aeiou".include?(c)
      Vowel.new(c)
    elsif c == 'y'   
      Y.new(c)
    else
      Consonant.new(c)
    end
  end
end

def translate_word(word)
  return nil if word.empty?
  w = assign_chars_to_classes(word)
  case w.first
  when Vowel
    w +=
      case w.last
      when Vowel 
        YAY
      when Y
        NAY
      else # Consonant
        AY
      end     
  else # begins with a consonant
    while w.first === Consonant
      w.rotate!
    end if w.find {|c| c === Vowel}
    w += AY   
  end
  w.first.capitalize! if word[0] == word[0].capitalize
  w.join
end 

p translate_word('')      # => nil
p translate_word('ask')   # => 'askyay'   
p translate_word('apple') # => 'appleyay'
p translate_word('any')   # => 'anynay'
p translate_word('d')     # => 'day'
p translate_word('dog')   # => 'ogday'
p translate_word('Phony') # => 'Onyphay'
p translate_word('zzzzi') # => 'izzzzay'
p translate_word('cat3')  # => 'Choked on cat3 (ArgumentError)'

Consider the word 'Phony'. The first step is

w = assign_chars_to_classes('Phony') # => ["p", "h", "o", "n", "y"]

where

w.map(&:class) # => [Consonant, Consonant, Vowel, Consonant, Y]

As w.first => "p" is of class Consonant (recall case uses ===), we see if w contains any vowels:

if w.find {|c| c === Vowel} # => "o"

As w contains at least one vowel, we move the leading consonants ("p" and "h") to the end:

while w.first === Consonant
  w.rotate!
end # => ["o", "n", "y", "p", "h"]

capitalize the first element of w if the word is capitalized:

w.first.capitalize! if word[0] == word[0].capitalize
  # w => ["O", "n", "y", "p", "h", "a", "y"]

and join the elements of w:

w.join # => "Onyphay" 

Edit: I initially had

w = w.chunk {|c| c.is_a? Consonant}.map(&:last).rotate.flatten + AY

in place of

while w.first === Consonant
  w.rotate!
end if w.find {|c| c === Vowel}

but prefer what I have now.