patternrubyMinor
Pig Latin Translator - follow-up
Viewed 0 times
translatorlatinfollowpig
Problem
About a week ago, I put up this code and got some really good help. I've modified it a bit and was seeking if someone would review my second iteration. Original can be found here.
Here's the new stuff. Basically, I stuck it all in a module, added support for the option of a dash or no dash. And gave it the ability to work with sentences.
Here's the new stuff. Basically, I stuck it all in a module, added support for the option of a dash or no dash. And gave it the ability to work with sentences.
module PigLatin
class Translator
#how all the words end
ENDING = "ay"
def initialize (options = {:dash => true})
@dash = options[:dash]
end
def translate (phrase)
sentences = phrase.split(".")
translated_sentences = sentences.map do |sentence|
translation(sentence) + "."
end
translated_sentences.join(" ")
end
private
def translation (phrase)
words = phrase.split
translated_words = words.map do |word|
if vowel_is_first(word)
translate_with_vowel(word)
else
translate_with_consonant(word)
end
end
translated_phrase = translated_words.join(" ")
if @dash == false
translated_phrase = translated_phrase.delete("-")
end
translated_phrase.capitalize
end
def vowel_is_first(word)
return true if word[0] =~ /a|e|i|o|u|A|E|I|O|U/
return false
end
def translate_with_vowel(word)
"#{word}-#{"w"+ENDING}"
end
def translate_with_consonant(word)
return "#{word[1]}-#{word[0]+ENDING}" if word.size == 2
second_segment, first_segment = split(word)
return "#{first_segment}-#{second_segment+ENDING}"
end
def split(word)
split_location = word =~ /a|e|i|o|u/
second_segment = word[0,split_location]
first_segment = word[split_location,word.size]
return second_segment, first_segment
end
end
class InputAcceptor
def initialize input, options
@options = options
@input = input
end
def do_it
pig_latin = Translator.new(@options)
pig_latin.translate(@input)
end
end
end
j = PigLatin::InputAcceptor.new("Hello. You. Guy.",{:dash => false})
p j.do_itSolution
Some points in addition to what checkorbored pointed out:
-
You should use character classes and the
-
You should get rid of the explicit
-
should really be written as
Except if
-
In this case it doesn't matter because there's only one option in your options hash, but the usual idiom to implement keyword arguments with defaults looks like this:
The reason that
-
You should use character classes and the
i flag for your regexen. E.g. instead of /a|e|i|o|u|A|E|I|O|U/ you can write /[aeiou]/i.-
You should get rid of the explicit
return statements where not necessary.-
return foo if bar
return bazshould really be written as
if bar
foo
else
baz
endExcept if
foo and baz are booleans in which case the whole thing can just be replaced by bar or !bar.-
def initialize (options = {:dash => true})
@dash = options[:dash]
endIn this case it doesn't matter because there's only one option in your options hash, but the usual idiom to implement keyword arguments with defaults looks like this:
def initialize (options = {})
options = {:dash => true}.merge(options)
@dash = options[:dash]
endThe reason that
(options = {defaults}) is generally not used, is that it doesn't allow the user to specify some options, but use the default for others. So if you do def bla(options = {:foo => 42, :bar => 23}) and the user calls bla(:bar => 24) the value for :foo will be nil, not 42. That doesn't happen if you use merge instead.Code Snippets
return foo if bar
return bazif bar
foo
else
baz
enddef initialize (options = {:dash => true})
@dash = options[:dash]
enddef initialize (options = {})
options = {:dash => true}.merge(options)
@dash = options[:dash]
endContext
StackExchange Code Review Q#2353, answer score: 4
Revisions (0)
No revisions yet.