My First Application

J

Jesús Gabriel y Galán

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

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

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=3D=3D1

- 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.
 
G

Greg Willits

Jason said:
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
 
J

Jason Hhh

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
 
J

Jason Hhh

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.
 
J

Jesús Gabriel y Galán

Jason said:
I have worked with PHP in the past and have been playing with Ruby for a
few weeks. =A0I made a program that I would love to get feedback on. =A0= Any
comments would be great. =A0I 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)

=A0numerals =A0 =A0 =3D ['M','D','C','L','X','V','I']
=A0values =A0 =A0 =A0 =3D [1000,500,100,50,10,5,1]
=A0implemented =A0 =3D []
=A0conversion =A0 =A0 =3D ""
=A0remainder =A0 =A0 =3D b10number

=A0values.each do |this_value|
=A0 =A0if this_value > 1
=A0 =A0 =A0implemented << remainder/this_value
=A0 =A0 =A0remainder =3D remainder % this_value
=A0 =A0else
=A0 =A0 =A0implemented << remainder
=A0 =A0end
=A0end

=A0implemented.each_with_index do |this_many, indx|
=A0 =A0conversion +=3D (numerals[indx] * this_many)
=A0end

=A0puts conversion
end

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

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

Jesus.
 
J

Jesús Gabriel y Galán

Jes=FAs Gabriel y Gal=E1n wrote:



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.
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.
Thanks for your your feedback Jesus.

BTW, nice job with your first application, looks really good for a first tr=
y.

Jesus.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,145
Messages
2,570,826
Members
47,372
Latest member
LucretiaFo

Latest Threads

Top