patternrubyMinor
English Translator - follow up
Viewed 0 times
englishfollowtranslator
Problem
Awhile ago I posted a Translator app that I have been in the process of making. I edited some features in it and I've decided to post it again.
My questions:
Here's the source code of the app:
```
require 'yaml'
require 'base64'
require 'digest'
def welcome_list
puts """
Welcome to Translation Center! Below you will find a list of available(so far) features, more feautures are being added as we speak! Have fun and enjoy!
As of now Translation Center is able to translate from:
1: Russian
2: Spanish
3: Arabic
4: Pashto
5: French
Translation Center is also able to translate into:
6: Base64
7: Binary
8: MD5
Please choose from the list above by typing in the number in front of the language:
"""
input = gets.chomp!
case input.to_i
when 1
translate
when 2
translate
when 3
translate
when 4
translate
when 5
translate
when 6
base64
when 7
binary
when 8
md5
else
puts "Invalid input, redirecting..."
return
end
end
def translate
translations = YAML.load_file 'words.yml'
puts "Enter word or phrase to be translated to English, press 'Q' to quit:"
input = gets.chomp
if translations[input]
puts "The translation of '#{input}' to English is: '#{translations[input]}'"
elsif input =~ /q/i
list
else
puts "Invalid word or phrase redirecting..."
translate
end
end
def binary
puts "Enter your text you wanted translated to binary:"
text = gets.chomp
puts "#{text} in binary is: #{text.unpack('b*')}"
binary if restart
end
def base64
puts "Enter the text you want translated to Base64:"
input = gets.chomp!.to_s
new_
My questions:
- Is there anything I should add to this?
- Is there any better (more easily readable) syntax for what I'm trying to do?
- Say I wanted to make a GUI for this app. What language goes well with Ruby?
Here's the source code of the app:
```
require 'yaml'
require 'base64'
require 'digest'
def welcome_list
puts """
Welcome to Translation Center! Below you will find a list of available(so far) features, more feautures are being added as we speak! Have fun and enjoy!
As of now Translation Center is able to translate from:
1: Russian
2: Spanish
3: Arabic
4: Pashto
5: French
Translation Center is also able to translate into:
6: Base64
7: Binary
8: MD5
Please choose from the list above by typing in the number in front of the language:
"""
input = gets.chomp!
case input.to_i
when 1
translate
when 2
translate
when 3
translate
when 4
translate
when 5
translate
when 6
base64
when 7
binary
when 8
md5
else
puts "Invalid input, redirecting..."
return
end
end
def translate
translations = YAML.load_file 'words.yml'
puts "Enter word or phrase to be translated to English, press 'Q' to quit:"
input = gets.chomp
if translations[input]
puts "The translation of '#{input}' to English is: '#{translations[input]}'"
elsif input =~ /q/i
list
else
puts "Invalid word or phrase redirecting..."
translate
end
end
def binary
puts "Enter your text you wanted translated to binary:"
text = gets.chomp
puts "#{text} in binary is: #{text.unpack('b*')}"
binary if restart
end
def base64
puts "Enter the text you want translated to Base64:"
input = gets.chomp!.to_s
new_
Solution
A few dozen teensy little things before I get to the meat of my review:
-
What on Earth is with this bit:
Why does anything containing Q -- no, not something that's just that letter, read the documentation -- run a method,
NOTE: In refactoring the whole program according to my description below, some of these things become invalid. However, they're still worth mentioning.
Right, on to the fun stuff. Namely, why your program is absolutely terrible in one teeny tiny edge case that you should have seen a mile away: Two languages that have the same word will seriously screw each other up. See, you divide your YAML file into human-readable pieces by doing a little header with the language, but YAML doesn't see it like that. What it sees is a key,
instead of
Now, fixing this is gonna cause a few problems, because you were relying on your bad way of doing things. First off,
Add a parameter,
Whew! That was an awful lot of effort. Now, that just broke all your code, because you're calling
Since the way I did it is pretty complicated, and involved an annoying amount of refactoring, I'm not gonna describe it in great detail. See the code if you want to know how. It's not that hard, just a different way of thinking.
There are a few remaining problems that I'm not going to fix, because I'm a lazy son of a you should practice. They are:
"""does not a multiline string make. It makes an empty string, a multiline string, and another empty string, all joined with newlines. Replace that with a heredoc -- this blog post goes into a good bit of detail about them.
- I can't find a source for it, but console windows are typically 80 characters wide. Anything more and you get wrapping, which may look fine on *nix but on Windows it looks awful, because it's character-based wrapping, not word-based.
- You made a bunch of grammar errors. I fixed 'em. Of course, most of the text could be reworked -- from the menu, you'd never guess that you can't translate to Russian, Spanish, Arabic, Pashto, or French, and you can't translate from base 64, binary, or MD5 hash. Well, maybe the last, but the rest it's not clear.
- In a
casestatement, you can see if a number is within a range by doingwhen lower_bound..upper_bound. Standard range syntax applies.
- You load the
YAMLfile every time someone wants to translate something. Load it once, then either make it a$globalvariable or pass it in throughtranslate's arguments.
- When you say "redirecting", you probably mean "please try again", since there isn't a program in the world that "redirects" to the same menu when you mess up the input. You also don't want to use recursion to do that, since at some apparently random point they'll get a
SystemStackErrorfor recursing too deeply. There's a reason that Ruby has iterative loops.
- On that note, why is it that when you enter invalid input anywhere else, you can try again, but in
welcome_list, the program says the same thing but then exits? Just remove the text and have it end, or drop a loop in like everything else.
- In
md5, why is the hash calledinput_string? It's very obviously not what they inputted. That should really be called something likehashorresult.
- Some of your English is inconsistent -- personally, I like it all to look the same, because it gives it a more clean feel if you see
'Enter the text you want translated to [language]:'and'abc' translated to [language] is: 'def'every time, instead of different text for each "language".
-
What on Earth is with this bit:
elsif input =~ /q/i
listWhy does anything containing Q -- no, not something that's just that letter, read the documentation -- run a method,
list, that doesn't exist? See, little teensy break-your-whole-program-no-matter-what bugs like that are why you have to test your code. I have no idea what you're trying to do here (i.e. go back to the welcome list or exit the program) so I assumed you wanted to exit the program. It's easier on me.- You use
gets.chomp!.to_s, which makes no sense, becausechomp!already returns a string, and the bang version (chomp!instead ofchomp) affects nothing in this case. You use it right elsewhere, so I'm confused.
NOTE: In refactoring the whole program according to my description below, some of these things become invalid. However, they're still worth mentioning.
Right, on to the fun stuff. Namely, why your program is absolutely terrible in one teeny tiny edge case that you should have seen a mile away: Two languages that have the same word will seriously screw each other up. See, you divide your YAML file into human-readable pieces by doing a little header with the language, but YAML doesn't see it like that. What it sees is a key,
language_name, that's mapped to nothing at all. If you wanna say "These are all the words in Russian" and have YAML associate "russian" with those words, you have to indent1. For example:russian:
А: A
В: Vinstead of
russian:
А: A
В: VNow, fixing this is gonna cause a few problems, because you were relying on your bad way of doing things. First off,
translate is depending on everything being in the bottom level. Instead, what you're gonna have to do is change it like this:Add a parameter,
all_words, and translations = all_words[language] before the first line.Whew! That was an awful lot of effort. Now, that just broke all your code, because you're calling
translate with only one argument, up above. What you're gonna wanna do is one of two things:- Hardcode your list into the
case, sort of like it was before. I recommend against this.
- Make it all super easy to change by having it sense what languages are available, update the list automatically, and put the appropriate options in the
case.
Since the way I did it is pretty complicated, and involved an annoying amount of refactoring, I'm not gonna describe it in great detail. See the code if you want to know how. It's not that hard, just a different way of thinking.
There are a few remaining problems that I'm not going to fix, because I'm a lazy son of a you should practice. They are:
- You mix input with calculation -- the calculation should ideally be separated from the input as
Code Snippets
elsif input =~ /q/i
listrussian:
А: A
В: Vrussian:
А: A
В: Vrequire 'yaml'
require 'base64'
require 'digest'
def welcome_list
word_bank = YAML.load_file 'words.yml'
languages = word_bank.keys
non_language_options = 3
i = non_language_options
puts <<-END.gsub(/^\s*>/, '')
>Welcome to Translation Center! Below you will find a list of as yet available features.
>More features are being added as we speak! Have fun and enjoy!
>
>Translation Center can translate to:
>
>1: Base64
>2: Binary
>3: MD5
>
>Translation Center can translate from:
>
>#{languages.map { |item| "#{i+=1}: #{item}" }.join "\n"}
>
>Please choose from the list above by typing in the number in front of the option:
END
input = gets.chomp
case choice = input.to_i
when 1
base64
when 2
binary
when 3
md5
when (non_language_options+1)..(languages.length+non_language_options+1)
translate(word_bank, languages[choice-non_language_options-1])
else
puts 'Invalid input.'
return
end
end
def translate(all_words, language)
translations = all_words[language]
while true
puts "Enter word or phrase to be translated from #{language} to English, press 'Q' to quit:"
input = gets.chomp
if translations[input]
puts "The translation of '#{input}' to English is: '#{translations[input]}'"
next
elsif input.upcase == 'Q'
exit
else
puts 'Invalid word or phrase. Please try again.'
next
end
break
end
end
def binary
while true
puts 'Enter the text you want translated to binary:'
text = gets.chomp
puts "#{text} in binary is: #{text.unpack('b*')}"
break unless restart
end
end
def base64
while true
puts 'Enter the text you want translated to Base64:'
input = gets.chomp
new_string = "#{input}"
puts "'#{input}' in Base64 is: #{Base64.encode64(new_string)}"
break unless restart
end
end
def md5
while true
puts 'Enter the text you want translated to MD5:'
input = gets.chomp
result = Digest::MD5.new
result.update(input)
puts "'#{input}' translated to MD5 is: '#{result}'"
break unless restart
end
end
def restart
puts <<-END.gsub(/^\s*>/, '')
>Would you like to:
>1. Translate another
>2. Go back to the main menu
>3. Quit
END
input = gets.chomp!
if input == '1'
true
elsif input == '2'
puts 'Redirecting to menu...'
welcome_list
else
puts 'Exiting...'
exit
end
end
welcome_listspanish:
a: to
abajo: down
arabic:
salam: Hi
ana min: I'm from...
russian:
А: A
В: V
french:
oui: Yes
non: No
pashto:
khe chare: Hello
japanese:
yaa: Hi
Yaa: Hi
Konbanw: Good eveningContext
StackExchange Code Review Q#109543, answer score: 4
Revisions (0)
No revisions yet.