Re: Help please Undefined Method error

On Fri, Feb 3, 2012 at 2:58 AM, andres d. <[e-mail]> wrote:

> The program is done !!!!  i was only checking it for the last time
> before going to do something else and I found the solution well the
> problem and the solution.
> the program now looks like this one
> Thank you for your help it was really usefull
>
> Attachments:
> http://www.ruby-forum.com/attachment/6949/sort_cor.rb
>
>
Hi Andes,

I took the liberty to propose an alternative to the
first block of your program. I am afraid some
aspects of your program are not really Ruby idiomatic.

This is meant as "constructive criticism", I hope you
can appreciate it. If not, sorry upfront.

<alternative>
def get_words
  [].tap do |a|
    while !(input = gets.chomp).empty?
       a << input
    end
  end
end

puts "Hello, enter all the words you want, finish with empty line"
words = get_words
puts "The words you entered are:"
puts words
</alternative>

Some highlights:

* put that functionality to read the words in a separate function
  and test it separately. Then you can also reuse it more easily.

* Use  [].tap{|a| a<< stuff}  so you do not have to return the
  result array explicitly (I am not having a variable "words"
  or "array" in the function, it is returned as the final result but
  not stored in an explicit local variable; so I can impossibly
  forget to return the built up array at the end, because "tap"
  returns it automatically

* don't do "while true ... break ... end"
  use "while (data is valid) ... do something with the data ... end"
  the "break" in your version burns in my eyes.

* take note of the special 'while !(input = gets.chomp)' construct
  with a _single_ '=' (not a double '==') in the condition for while.
  Some people may find it not optimal to make an assignment,
  but it is the style I prefer for that use case. I am open to alternatives
  for that ...

* don't store the data that is not valid (the empty string) in the array
  and then delete it afterwards. Evaluate the validity of the data
  _before_ storing it in the array.

* a more advanced aspect is that inside the block of "tap", there
  is no variable used that comes from the outside of the block.
  That makes for safer code as there is less chance for accidental
  dependency of a variable that is set higher-up in the code.
  This is also the main reason for short functions, less chance for
  unintended dependencies on far-away variables.

Sorry for being critical, but I honestly hope you can take the time
to analyze my comments and learn some neat "Ruby" tricks from it.

Success with Ruby and welcome :-)

Peter