My First Application

ruby-talk
1

I have worked with PHP in the past and have been playing with Ruby for a
few weeks. I made a program that I would love to get feedback on. Any
comments would be great. I have my program on Git hub.

http://github.com/jasonhamilton/NZBMatrix-Downloader

···

--
Posted via http://www.ruby-forum.com/.

2

Quick feedback:

- Variables that start with $ are global variables. Try to avoid them,
and specially in a program like this I think they are not needed. Make
them local variables, and replace the few that you use inside the
class with an instance variable that you set when creating the
instance.

- For an infinite loop: loop do {}, instead of while 1==1

- I can't see how you exit the loop

- In the loop body, there's too much detailed processing of the
message, calling to_s, downcase and indexing several times. There's
probably a way to DRY that up, but I'm in a rush right now.

If I have more time later I will comment a bit more.

Hope this helps,

Jesus.

···

On Mon, Sep 6, 2010 at 6:01 AM, Jason Hhh <jasonscotthamilton@gmail.com> wrote:

I have worked with PHP in the past and have been playing with Ruby for a
few weeks. I made a program that I would love to get feedback on. Any
comments would be great. I have my program on Git hub.

http://github.com/jasonhamilton/NZBMatrix-Downloader

3

Jason Hhh wrote:

I have worked with PHP in the past and have been playing with Ruby for a
few weeks. I made a program that I would love to get feedback on. Any
comments would be great. I have my program on Git hub.

http://github.com/jasonhamilton/NZBMatrix-Downloader

Well, the conversion isn't accurate yet, but using your approach, here's
another way to do it that allows you to easily grow the list of numerals
to be converted:

def convert(b10number)

  numerals = ['M','D','C','L','X','V','I']
  values = [1000,500,100,50,10,5,1]
  implemented =
  conversion = ""
  remainder = b10number

  values.each do |this_value|
    if this_value > 1
      implemented << remainder/this_value
      remainder = remainder % this_value
    else
      implemented << remainder
    end
  end

  implemented.each_with_index do |this_many, indx|
    conversion += (numerals[indx] * this_many)
  end

  puts conversion
end

puts 'Howdy there, enter a number and I\'ll convert it to the older
Roman Numeral fashion!'
argument = gets.chomp.to_i
convert argument

···

--
Posted via http://www.ruby-forum.com/\.

4

Well, the conversion isn't accurate yet, but using your approach, here's
another way to do it that allows you to easily grow the list of numerals
to be converted:

Thanks Greg

···

--
Posted via http://www.ruby-forum.com/\.

5

Jesús Gabriel y Galán wrote:

- Variables that start with $ are global variables. Try to avoid them,
and specially in a program like this I think they are not needed. Make
them local variables, and replace the few that you use inside the
class with an instance variable that you set when creating the
instance.

How can I get the variables into the class then?

- For an infinite loop: loop do {}, instead of while 1==1

- I can't see how you exit the loop

I would like the user to be able to exit the loop by pressing a button
or something like that but I'm not sure how to do that without messing
up the looping.

- In the loop body, there's too much detailed processing of the
message, calling to_s, downcase and indexing several times. There's
probably a way to DRY that up, but I'm in a rush right now.

Thanks for your your feedback Jesus.

···

--
Posted via http://www.ruby-forum.com/\.

6

Greg, I think you answered the wrong thread...

Jesus.

···

On Mon, Sep 6, 2010 at 11:23 AM, Greg Willits <lists@gregwillits.ws> wrote:

Jason Hhh wrote:

I have worked with PHP in the past and have been playing with Ruby for a
few weeks. I made a program that I would love to get feedback on. Any
comments would be great. I have my program on Git hub.

http://github.com/jasonhamilton/NZBMatrix-Downloader

Well, the conversion isn't accurate yet, but using your approach, here's
another way to do it that allows you to easily grow the list of numerals
to be converted:

def convert(b10number)

numerals = ['M','D','C','L','X','V','I']
values = [1000,500,100,50,10,5,1]
implemented =
conversion = ""
remainder = b10number

values.each do |this_value|
if this_value > 1
implemented << remainder/this_value
remainder = remainder % this_value
else
implemented << remainder
end
end

implemented.each_with_index do |this_many, indx|
conversion += (numerals[indx] * this_many)
end

puts conversion
end

puts 'Howdy there, enter a number and I\'ll convert it to the older
Roman Numeral fashion!'
argument = gets.chomp.to_i
convert argument

7

Jesús Gabriel y Galán wrote:

- Variables that start with $ are global variables. Try to avoid them,
and specially in a program like this I think they are not needed. Make
them local variables, and replace the few that you use inside the
class with an instance variable that you set when creating the
instance.

How can I get the variables into the class then?

Pass them in the initialize method if they are variable or use
Constants inside the class, if they are harcoded values.

- For an infinite loop: loop do {}, instead of while 1==1

- I can't see how you exit the loop

I would like the user to be able to exit the loop by pressing a button
or something like that but I'm not sure how to do that without messing
up the looping.

If you have some work to do that you don't want to stop to ask the
user for input, you should use threads or processes. You launch a
thread or process to do the work and present a menu to the user in the
main thread to control the job.

- In the loop body, there's too much detailed processing of the
message, calling to_s, downcase and indexing several times. There's
probably a way to DRY that up, but I'm in a rush right now.

Thanks for your your feedback Jesus.

BTW, nice job with your first application, looks really good for a first try.

Jesus.

···

On Mon, Sep 6, 2010 at 6:32 PM, Jason Hhh <jasonscotthamilton@gmail.com> wrote: