4
\$\begingroup\$

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:

  1. Is there anything I should add to this?
  2. Is there any better (more easily readable) syntax for what I'm trying to do?
  3. 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_string = "#{input}"
    puts "'#{input}' in Base64 is: #{Base64.encode64(new_string)}"  
    base64 if restart
end

def md5
    puts "Enter text you want translated to MD5:"
    input = gets.chomp!.to_s
    input_string = Digest::MD5.new
    input_string.update(input)
    puts "'#{input}' translated to MD5: '#{input_string}'"
    md5 if restart
end

def restart
    puts "Would you like to [1]translate another, go back to the [2]main menu, or [3]quit?"
    input = gets.chomp!
     if input == "1"
        return true
    elsif input == "2"
        puts "Redirecting to Menu..."
        welcome_list
    else
        puts "Exiting system..."
        exit
    end
end
welcome_list

Sample words.yml file:

spanish: 
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 evening
\$\endgroup\$
  • \$\begingroup\$ May I suggest perhaps just including a link to the YAML file instead of posting the whole file in a code block? Since it is not code per-se, it doesn't bring that much value from a code review perspective to have the whole file in the post, I hope you get some great answers! \$\endgroup\$ – Phrancis Nov 2 '15 at 15:02
  • \$\begingroup\$ @Phrancis That's not a bad idea lol. I might just take it out since it really has nothing to so with the code itself. \$\endgroup\$ – 13aal Nov 2 '15 at 15:11
  • 2
    \$\begingroup\$ @LostBam (Maybe a little bit of the structure of the YAML, that can be of help) \$\endgroup\$ – Quant Nov 2 '15 at 15:27
  • \$\begingroup\$ @Quant Added the structure of the YAML \$\endgroup\$ – 13aal Nov 2 '15 at 15:37
4
\$\begingroup\$

A few dozen teensy little things before I get to the meat of my review:

  1. """ 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.
  2. 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.
  3. 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.
  4. In a case statement, you can see if a number is within a range by doing when lower_bound..upper_bound. Standard range syntax applies.
  5. You load the YAML file every time someone wants to translate something. Load it once, then either make it a $global variable or pass it in through translate's arguments.
  6. 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 SystemStackError for recursing too deeply. There's a reason that Ruby has iterative loops.
  7. 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.
  8. In md5, why is the hash called input_string? It's very obviously not what they inputted. That should really be called something like hash or result.
  9. 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".
  10. What on Earth is with this bit:

    elsif input =~ /q/i
      list
    

    Why 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.

  11. You use gets.chomp!.to_s, which makes no sense, because chomp! already returns a string, and the bang version (chomp! instead of chomp) 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
    В: V

instead of

russian:
А: A
В: V

Now, 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:

  1. Hardcode your list into the case, sort of like it was before. I recommend against this.
  2. 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:

  1. You mix input with calculation -- the calculation should ideally be separated from the input as much as is feasible. That is, if you're using puts and gets in the same method as the one where you perform a calculation, separate them. Note that this isn't always true -- if the calculation is solely for the purpose of dynamic text, for example, you shouldn't separate that out. Probably. It depends.
  2. You made it almost impossible to easily loop the whole program, instead of just bits, because you focused on using recursion.

    Note: As a side effect of this, I couldn't easily figure out how to make restart work without recursion, so I left it alone.

  3. In translate, if the word happens to be Q or q (which is unlikely but possible), you won't give the translation. You need to listen for CTRL+C (i.e. InterruptError if memory serves) and quit on that, then say that you're quitting on that.
  4. If they enter the text in anything but all lowercase, it doesn't give them the translation, unless you put the uppercase versions in the YAML manually.
  5. binary prints out the result as ["100001101000011010000110"] instead of 100001101000011010000110 (for the string aaa).

(mostly) Fixed code:

require '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_list

Fixed words.yml:

spanish:
    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 evening

As for a GUI, I suggest Shoes!, because it's conveniently already in Ruby.


1: Little teensy things like this are why you always, always, always, always read the documentation for everything you're using. ALWAYS.

\$\endgroup\$
  • \$\begingroup\$ First off, by far the best most detailed easily followed answer I've ever gotten anywhere on stack if I could vote this answer up a dozen times I would. Secondly, this was my first attempt wit ha YAML, thank you the input on it, I will fix it. Finally "I'm a lazy son of a" brought a tear to my eye, thank you again lol. I'm actually planning on rewriting the entire thing using JRuby, for a GUI. \$\endgroup\$ – 13aal Nov 11 '15 at 20:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.