I'm looking to get some feedback on this program I wrote - ways I could make it simpler or best practices methods.
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(', ') + "."
-
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\$Wayne Conrad– Wayne Conrad2014年05月12日 05:14:35 +00:00Commented May 12, 2014 at 5:14
2 Answers 2
As Wayne Conrad pointed out in the comments, you should really use
snake_case
in Ruby. I'd also just call the arraywords
instead ofinput_array
. Whileinput_array
describes what sort of variable it is, it doesn't describe its content, or hint at its purpose.The Ruby convention is 2 spaces of indentation. Not a tab, not 4 spaces.
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".
use
String#empty?
anduntil
instead of "manually" comparing against an empty stringUse the shovel operator
<<
instead ofpush
. They do the same thing (exceptpush
can take several arguments), but the shovel operator is the more common way of doing things.Why use
print
in the last line? Useputs
instead, so you'll get a newline after your program exits.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 }
-
\$\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\$Jimmy– Jimmy2014年05月12日 22:15:31 +00:00Commented 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\$Flambino– Flambino2014年05月13日 08:21:03 +00:00Commented May 13, 2014 at 8:21
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(', ')}.")
-
1\$\begingroup\$ I always forget about #take_while. Very nice! \$\endgroup\$Wayne Conrad– Wayne Conrad2014年05月12日 17:45:36 +00:00Commented May 12, 2014 at 17:45
-
\$\begingroup\$ @WayneConrad Ditto :) \$\endgroup\$Flambino– Flambino2014年05月13日 08:08:43 +00:00Commented May 13, 2014 at 8:08