6
\$\begingroup\$

I'm looking to get some feedback on this program I wrote - ways I could make it simpler or best practices methods.

Chapter 7:

Let's write a program which asks us to type in as many words as we want (one word per line, continuing until we just press Enter on an empty line), and which then repeats the words back to us in alphabetical order. OK?

...

Assignment: Write the program we talked about at the very beginning of this chapter. Hint: There's a lovely array method which will give you a sorted version of an array: sort. Use it!

Here's the program I wrote:

puts "This program will take your words and sort them alphabetically"
puts ""
puts "Type in a word and press enter. When you are done, press enter on an empty line to launch program."
inputArray = []
while (inputWord = gets.chomp) != ""
 inputArray.push inputWord
 puts "Current List is: " + inputArray.join(', ')
end
print "Your list in alphabetical order is: " + inputArray.sort.join(', ') + "."
asked May 11, 2014 at 22:28
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Ruby prefers snake_case over camelCase for variable names. That's all I see. Is the program's I/O dictated by the requirement, or can it be changed? I ask because the program could be made simpler if, for example, EOF could signal the end of input rather than a blank line. \$\endgroup\$ Commented May 12, 2014 at 5:14

2 Answers 2

3
\$\begingroup\$
  1. As Wayne Conrad pointed out in the comments, you should really use snake_case in Ruby. I'd also just call the array words instead of input_array. While input_array describes what sort of variable it is, it doesn't describe its content, or hint at its purpose.

  2. The Ruby convention is 2 spaces of indentation. Not a tab, not 4 spaces.

  3. Use a heredoc for the intro text:

    puts <<END_INFO
    This program will take your words and sort them alphabetically
    Type in a word and press enter. When you are done, press enter on an empty line to launch program.
    END_INFO
    

    While we're on the subject of the info text: The second line doesn't make much sense. The program is already running when you see that message; there's nothing to "launch".

  4. use String#empty? and until instead of "manually" comparing against an empty string

  5. Use the shovel operator << instead of push. They do the same thing (except push can take several arguments), but the shovel operator is the more common way of doing things.

  6. Why use print in the last line? Use puts instead, so you'll get a newline after your program exits.

  7. Use string interpolation ("#{...}") for simple stuff

I end up with this

puts <<INFO
This program will take your words and sort them alphabetically
Type in a word and press enter. When you are done, press enter on an empty line to launch program.
INFO
words = []
until (word = gets.chomp).empty?
 words << word
 puts "Current list is: #{words.join(", ")}."
end
puts "Your list in alphabetical order is: #{words.sort.join(", ")}."

You can even reduce it down to:

[].tap do |words|
 until (word = gets.chomp).empty?
 words << word
 puts "Current List is: #{words.join(", ")}."
 end
end.tap do |words|
 puts "Your list in alphabetical order is: #{words.sort.join(", ")}."
end

Or even this (which still completes the stated task):

[].tap do |words|
 until (word = gets.chomp).empty?
 words << word
 end
end.sort.each { |word| puts word }
answered May 12, 2014 at 14:00
\$\endgroup\$
2
  • \$\begingroup\$ Thanks Flambino! A ton of good points that I hadn't seen before. I'm fairly new to ruby so I don't know all the best practices yet. 1. The snake case is a good call, I've had some experience in java so I was very used to camel case. 2. Sublime Text creates the tab indentation for me but is it fairly standard to have only 2 indents? 3. Yes! I've seen << been used over push so thanks for confirming that it's standard. 4.Haven't really touched on string interpolation yet, i'll have to look into that, but it looks extremely useful. Thanks again! \$\endgroup\$ Commented May 12, 2014 at 22:15
  • \$\begingroup\$ @Jimmy The 2-spaces indentation is the Ruby standard, really (see the Ruby docs, for instance, or something like this style guide). You can configure Sublime's (and any other proper editor) indentation any way you want. In Sublime's case, there's a menu in the bottom of the window, as I recall, which'll let you set the width and type of indentation it should use. \$\endgroup\$ Commented May 13, 2014 at 8:21
5
\$\begingroup\$

Flambino has already pointed out the issues in your code, so I'll just show how to tackle the problem with a different approach. A declarative and functional code (as opposed to imperative) favours immutability and known abstractions over manual control flow and in-place updates. Uusing lazy from Ruby 2 and String#present? from active_support (so we can write the more declarative code we can) it would look like this:

require 'active_support/core_ext/string'
sorted_words = $stdin.lines.lazy.map(&:strip).take_while(&:present?).sort
puts("Your list in alphabetical order is: #{sorted_words.join(', ')}.")
answered May 12, 2014 at 17:12
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I always forget about #take_while. Very nice! \$\endgroup\$ Commented May 12, 2014 at 17:45
  • \$\begingroup\$ @WayneConrad Ditto :) \$\endgroup\$ Commented May 13, 2014 at 8:08

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.