Review code, refactoring

Hi,

Is it possible to do this simpler ?

--8<--

# Check if your input is a number, quit if input is equal to 'quit'
class Testing_input_number
   def initialize
     answer = nil
     while a_number?(answer) != true
       answer = ask
       puts 'Please, a number.' unless a_number?(answer)
     end
     puts "OK, this is a number."
   end

   def ask
     print "Try?\s"
     number = gets.chomp
     bybye if number == 'quit'
     number
   end

   def a_number?(number)
     number.to_s == number.to_i.to_s
   end

   def bybye
     puts "Goodbye."
     exit
   end
end

player = Testing_input_number.new

-->8--

Thank you very much for the review.

/Nathan

You don't need a class for this. Just use an until loop.

is_number = false

until is_number
  print "Enter a number: "
  raw_number = gets.chomp

  exit if raw_number == 'quit'

  is_number = number.to_s == number.to_i.to_s
end

puts "OK, this is a number."

···

On Tue, Nov 29, 2016 at 10:44 PM, Aleksey Ivanov <ialexxei@gmail.com> wrote:

Hi.

First thing that can be noticed is to replace

    while smth != true do
      ...
    end

with

    until smth do
      ...
    end

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

The 'break value' statement is another option:

result = while true do
  print "Enter a number: "
  input = gets.chomp
  if input == "quit"
    exit
  elsif input.to_i.to_s == input
    break input.to_i
  end
end

puts "You entered #{result}"

Calling "break" with a value makes that value the return value of the loop,
which is what you want here.

martin

Quoting Maxim Hedrovich (maximhedrovich@gmail.com):

> class name need to write in CamelCase
> in your case "class TestingInputNumber"

No!!

Only the first letter needs to be capital!

class Testing_input_number is perfectly legal & acceptable.

1. According to "the" Ruby Style Guide (GitHub - rubocop/ruby-style-guide: A community-driven Ruby coding style guide), you should use CamelCase for classes.

2. I don't agree with the Style Guide on a number of things*. (I happen to agree about this, but that's not relevant here.)

3. I personally think you should use whatever you are comfortable with -- always considering the question "will this drive other people that have to maintain the code insane?". There is no "legal" or "acceptable": the code is *yours*.

4. The really important thing is to be consistent.

[ *I've just spotted "all names should be in English"... I wrote something mildly vitriolic here, then deleted it.]

Click here to view Company Information and Confidentiality Notice.<http://www.jameshall.co.uk/index.php/small-print/email-disclaimer&gt;

Quoting Maxim Hedrovich (maximhedrovich@gmail.com):

   class name need to write in CamelCase
   in your case "class TestingInputNumber"

No!!

Only the first letter needs to be capital!

class Testing_input_number is perfectly legal & acceptable.

(I really do not like camel casing...)

Carlo

···

Subject: Re: Review code, refactoring
  Date: Wed 30 Nov 16 10:25:45AM +0300

--
  * Se la Strada e la sua Virtu' non fossero state messe da parte,
* K * Carlo E. Prelz - fluido@fluido.as che bisogno ci sarebbe
  * di parlare tanto di amore e di rettitudine? (Chuang-Tzu)

Hi.

First thing that can be noticed is to replace

    while smth != true do
      ...
    end

with

    until smth do
      ...
    end

class name need to write in CamelCase
in your case "class TestingInputNumber"

···

2016-11-30 9:46 GMT+03:00 Brandon Weaver <keystonelemur@gmail.com>:

You don't need a class for this. Just use an until loop.

is_number = false

until is_number
  print "Enter a number: "
  raw_number = gets.chomp

  exit if raw_number == 'quit'

  is_number = number.to_s == number.to_i.to_s
end

puts "OK, this is a number."

On Tue, Nov 29, 2016 at 10:44 PM, Aleksey Ivanov <ialexxei@gmail.com> > wrote:

Hi.

First thing that can be noticed is to replace

    while smth != true do
      ...
    end

with

    until smth do
      ...
    end

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

Many thanks for all that reply to my question.

So, i tried this (using the Martin's example):

--8<--

result = while true do
          print 'Try? '
          input = gets.chomp
          exit if input == 'quit'
          break input.to_i if input.to_i.to_s == input
          puts "Please, a number."
          end

puts "You entered #{result}"

-->8--

It works, but checking with rubocop :

--8<--

debug.rb:1:1: C: Carriage return character detected.
result = while true do ...
^^^^^^^^^^^^^^^^^^^^^^
debug.rb:1:10: C: Use Kernel#loop for infinite loops.
result = while true do
          ^^^^^
debug.rb:1:16: W: Literal true appeared in a condition.
result = while true do
                ^^^^
debug.rb:1:21: C: Do not use do with multi-line while.
result = while true do
                     ^^
debug.rb:2:10: C: Use 2 (not 0) spaces for indentation.
          print 'Try? '

debug.rb:6:15: C: Prefer single-quoted strings when you don't need string interpolation or special symbols.
          puts "Please, a number."
               ^^^^^^^^^^^^^^^^^^^
debug.rb:10:1: C: 1 trailing blank lines detected.

1 file inspected, 7 offenses detected

-->8--

Can you help me to troubleshoot this ?
I use also codebeat to review code, it accepts less than 10 lines per method.
How can i refactor that ?

Thank you very much for all your replies.

Regards,

/Nathan

···

Le 2016-11-30 12:14, Martin DeMello a écrit :

The 'break value' statement is another option:

result = while true do
print "Enter a number: "
input = gets.chomp
if input == "quit"
exit
elsif input.to_i.to_s == input
break input.to_i
end

puts "You entered #{result}"

Calling "break" with a value makes that value the return value of the loop, which is what you want here.

martin

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

​Quite the assertion, there.​

···

On 30 November 2016 at 17:25, Maxim Hedrovich <maximhedrovich@gmail.com> wrote:

class name need to write in CamelCase
in your case "class TestingInputNumber"

--
  Matthew Kerwin
  http://matthew.kerwin.net.au/

If this is some kind of REPL you could do

def work_with_number(i)
  printf "Got: %10d\n", i
end

loop do
  print "Your input, please: "
  input = gets.chomp

  case input
  when /\A\s*(\d+)\s*\z/
    work_with_number(Integer($1))
  when /\A\s*quit\s*\z/
    puts "Terminating."
    break
  else
    $stderr.puts "ERROR: invalid input: #{input}"
  end
end

If you only want to read an integer, you could do

def input_number
  begin
    print "Input a number, please: "
    Integer(gets.chomp)
  rescue ArgumentError => e
    puts e # "Not a number"
    retry
  end
end

i = input_number

and the result of this function is the integer value

Kind regards

robert

···

On Wed, Nov 30, 2016 at 7:36 AM, Nathan Guilty <ruby@e-solutions.re> wrote:

Hi,

Is it possible to do this simpler ?

--8<--

# Check if your input is a number, quit if input is equal to 'quit'
class Testing_input_number
  def initialize
    answer = nil
    while a_number?(answer) != true
      answer = ask
      puts 'Please, a number.' unless a_number?(answer)
    end
    puts "OK, this is a number."
  end

  def ask
    print "Try?\s"
    number = gets.chomp
    bybye if number == 'quit'
    number
  end

  def a_number?(number)
    number.to_s == number.to_i.to_s
  end

  def bybye
    puts "Goodbye."
    exit
  end
end

player = Testing_input_number.new

--
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can
- without end}
http://blog.rubybestpractices.com/

Many thanks for all that reply to my question.

So, i tried this (using the Martin's example):

--8<--

result = while true do
        print 'Try? '
        input = gets.chomp
        exit if input == 'quit'
        break input.to_i if input.to_i.to_s == input
        puts "Please, a number."
        end

puts "You entered #{result}"

-->8--

It works, but checking with rubocop :

--8<--

debug.rb:1:1: C: Carriage return character detected.
result = while true do ...
^^^^^^^^^^^^^^^^^^^^^^
debug.rb:1:10: C: Use Kernel#loop for infinite loops.
result = while true do
        ^^^^^
debug.rb:1:16: W: Literal true appeared in a condition.
result = while true do
              ^^^^
debug.rb:1:21: C: Do not use do with multi-line while.
result = while true do
                   ^^
debug.rb:2:10: C: Use 2 (not 0) spaces for indentation.
        print 'Try? '

debug.rb:6:15: C: Prefer single-quoted strings when you don't need string interpolation or special symbols.
        puts "Please, a number."
             ^^^^^^^^^^^^^^^^^^^
debug.rb:10:1: C: 1 trailing blank lines detected.

1 file inspected, 7 offenses detected

Have you tried rubocop’s autocorrect option (--auto-correct) to see what it would prefer? Be aware that http://rubocop.readthedocs.io/en/latest/basic_usage/ says "Note: Experimental - use with caution.”, so make sure you have a backup of the file.

Rubocop also has an --extra-details flag which gives more details!

Hope this helps,

Mike

···

On Nov 30, 2016, at 6:42 AM, Nathan Guilty <ruby@e-solutions.re> wrote:

-->8--

Can you help me to troubleshoot this ?
I use also codebeat to review code, it accepts less than 10 lines per method.
How can i refactor that ?

Thank you very much for all your replies.

Regards,

/Nathan

Le 2016-11-30 12:14, Martin DeMello a écrit :

The 'break value' statement is another option:
result = while true do
print "Enter a number: "
input = gets.chomp
if input == "quit"
exit
elsif input.to_i.to_s == input
break input.to_i
end
end
puts "You entered #{result}"
Calling "break" with a value makes that value the return value of the loop, which is what you want here.
martin
Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;

--

Mike Stok <mike@stok.ca>
http://www.stok.ca/~mike/

The "`Stok' disclaimers" apply.

It's saying that you shouldn't use the "while true do ... end" construct if
you want an infinite loop, you should just use "loop do ... end". (Which is
correct; I'd forgotten about "loop" :))

Simply replace the first line with

result = loop do

and it should remove that warning. The second warning is that "do" is
actually redundant in a while loop (live and learn!); instead of

while condition do
  ...
end

you can simply say

while condition
  ...
end

The third warning is just a matter of correct indentation. The final one
hinges on a semantic difference in single versus double quoted ruby string
literals - a single-quoted string will literally reproduce its contents,
whereas a double-quoted one will perform string interpolation if you have a
#{} block within it, and interpret escape characters like \n.

$ irb
a = irb(main):001:0> a = 'hello'
=> "hello"
irb(main):002:0> b = '#{a} world'
=> "\#{a} world"
irb(main):003:0> c = "#{a} world"
=> "hello world"

There is no real reason to heed that warning - see

martin

···

On Wed, Nov 30, 2016 at 3:42 AM, Nathan Guilty <ruby@e-solutions.re> wrote:

Many thanks for all that reply to my question.

So, i tried this (using the Martin's example):

--8<--

result = while true do
         print 'Try? '
         input = gets.chomp
         exit if input == 'quit'
         break input.to_i if input.to_i.to_s == input
         puts "Please, a number."
         end

puts "You entered #{result}"

-->8--

It works, but checking with rubocop :

--8<--

debug.rb:1:1: C: Carriage return character detected.
result = while true do ...
^^^^^^^^^^^^^^^^^^^^^^
debug.rb:1:10: C: Use Kernel#loop for infinite loops.
result = while true do
         ^^^^^
debug.rb:1:16: W: Literal true appeared in a condition.
result = while true do
               ^^^^
debug.rb:1:21: C: Do not use do with multi-line while.
result = while true do
                    ^^
debug.rb:2:10: C: Use 2 (not 0) spaces for indentation.
         print 'Try? '

debug.rb:6:15: C: Prefer single-quoted strings when you don't need string
interpolation or special symbols.
         puts "Please, a number."
              ^^^^^^^^^^^^^^^^^^^
debug.rb:10:1: C: 1 trailing blank lines detected.

1 file inspected, 7 offenses detected

-->8--

Can you help me to troubleshoot this ?
I use also codebeat to review code, it accepts less than 10 lines per
method.
How can i refactor that ?

Thank you very much for all your replies.

Regards,

/Nathan

Le 2016-11-30 12:14, Martin DeMello a écrit :

The 'break value' statement is another option:

result = while true do
print "Enter a number: "
input = gets.chomp
if input == "quit"
exit
elsif input.to_i.to_s == input
break input.to_i
end
end

puts "You entered #{result}"

Calling "break" with a value makes that value the return value of the
loop, which is what you want here.

martin

Unsubscribe: <mailto:ruby-talk-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk&gt;