The Odin Project - Project Ruby: Translating from English to Pig Latin

Question

I just finished the pig latin translator for The Odin Project and I would appreciate some honest feedback for my code. The code is intended to be "test-driven learning", and therefore emulates test driven development.

Here is the prompt:

Pig Latin is a made-up children's language that's intended to be confusing. It obeys a few simple rules (below) but when it's spoken quickly it's really difficult for non-children (and non-native speakers) to understand.

Rule 1: If a word begins with a vowel sound, add an "ay" sound to the end of the word.

Rule 2: If a word begins with a consonant sound, move it to the end of the word, and then add an "ay" sound to the end of the word.

(There are a few more rules for edge cases, and there are regional variants too, but that should be enough to understand the tests.)

Here is the test spec:

require "pig_latin" describe "#translate" do it "translates a word beginning with a vowel" do s = translate("apple") expect(s).to eq("appleay") end it "translates a word beginning with a consonant" do s = translate("banana") expect(s).to eq("ananabay") end it "translates a word beginning with two consonants" do s = translate("cherry") expect(s).to eq("errychay") end it "translates two words" do s = translate("eat pie") expect(s).to eq("eatay iepay") end it "translates a word beginning with three consonants" do expect(translate("three")).to eq("eethray") end it "counts 'sch' as a single phoneme" do s = translate("school") expect(s).to eq("oolschay") end it "counts 'qu' as a single phoneme" do s = translate("quiet") expect(s).to eq("ietquay") end it "counts 'qu' as a consonant even when it's preceded by a consonant" do s = translate("square") expect(s).to eq("aresquay") end it "translates many words" do s = translate("the quick brown fox") expect(s).to eq("ethay ickquay ownbray oxfay") end # Test-driving bonus: # * write a test asserting that capitalized words are still capitalized (but with a different initial capital letter, of course) # * retain the punctuation from the original phrase end

Finally, here is my code:

def translate(phrase) #translates individual words or multiple words into pig latin new_phrase = [] if phrase.include? " " new_phrase = phrase.split(" ").map do |word| rearrange(word) end new_phrase = new_phrase.join(" ") else new_phrase = rearrange(phrase) end new_phrase end def rearrange(word) #rearranges individual words into pig latin vowels = ["a", "e", "i", "o", "u", "y"] new_word = word word.each_char do |char| if char == "u" && new_word[-1] == "q" new_word << "u" new_word.slice!(0) next elsif vowels.include? char new_word << "ay" break else new_word << char new_word.slice!(0) next end end new_word end

tokland · Accepted Answer · 2016-11-14 18:27:00Z

Some notes:

expect(translate("three")).to eq("eethray"). Be consistent, use always a local variable or don't. I think this way looks pretty cool.
Isn't the code inside a module/class?
translate. This methods is unnecessarily verbose. You don't need to check if there is a space or not, since the algorithm works just the same if there is not.
Those two nexts are redundant.
rearrange: This method would look so much nicer if you don't use imperative style.
If a regular expression can express the rules, you should consider using it. It will be easier to read and much more compact.
Side note: The rules are pretty simple. If they ever get more complex, a manual processing will be undecipherable and regexps will come short. Then you'll need something more sophisticated, like a parser (i.e. grammy).

I'd write:

def translate(phrase) phrase.split(" ").map { |word| rearrange(word) }.join(" ") end def rearrange(word) match = word.match(/^((?:qu|[bcdfghjklmnpqrstvwxz])*)(.*)$/) match ? match[2] + match[1] + "ay" : word end

If you have trouble reading that regular expressions, consider using multi-line regexps with named captures:

def rearrange(word) match = word.match(%r{ ^ (?<leading_consonants>(?:qu|[bcdfghjklmnpqrstvwxz])*) (?<rest>.*) $) match ? match[:rest] + match[:leading_consonants] + "ay" : word end

Thank you! I kinda figured that the methods were too verbose, I appreciate the feedback. — Adrian DeRose
– Adrian DeRose, Commented Nov 12, 2016 at 1:42

Stack Exchange Network

The Odin Project - Project Ruby: Translating from English to Pig Latin

1 Answer 1

You must log in to answer this question.

Hot Network Questions

The Odin Project - Project Ruby: Translating from English to Pig Latin

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions