I am new to programming and I would like some feedback. This is my first ruby program that simply adds a + b. I know it's probably more complicated than it needs to be, but I wanted to implement ideas that I was learning. Is there anything that I should do that I didn't or did that I shouldn't?
puts "<<<-----ADDITION----->>>"
puts "<<<------A+B=C------->>>"
letters = %i(a b)
sum = 0
letters.each do |let|
define_method(let) do
begin
puts "Enter #{let}"
let = Float(gets.chomp)
sum = sum + let.to_f
rescue
puts "Something went wrong. Try again."
retry
end
end
end
letters.each do |let|
send(let)
end
puts sum
-
5\$\begingroup\$ I'm torn; on the one hand, this is waaaay more complicated than it has any reason to be; by that metric, it's pretty 'terrible' code. On the other hand, given the goal of just using a bunch of crazy techniques for fun, the implementation is actually pretty cool. I don't have time at the moment, but I'll probably come back and leave a review later when I get a chance. \$\endgroup\$philomory– philomory2016年08月10日 04:01:37 +00:00Commented Aug 10, 2016 at 4:01
3 Answers 3
As philomory said in a comment, this is a neat demonstration of some of Ruby's features, but also a horribly complex way of saying a + b
.
In it's most basic formulation, you could just do:
puts gets.chomp.to_f + gets.chomp.to_f
Of course, this leaves out a lot of the stuff your code is currently doing (like telling the user what they're supposed to input). So let's see if we can get all the same functionality, but with a different structure.
First, though, a few notes on your current code (apart from the metaprogramming):
Don't overwrite arguments just to avoid a new variable. You pass
let
(a symbol) todefine_method
, but then use it to store the user's chosen number. So now you have a variable namedlet
, presumably short for "letter", which has nothing to do with letters - now it's a floating point number. You have the contents (the input), but you've overwritten its label. Or, you have the answer, but you've forgotten the question, to paraphrase Hitchhiker's Guide.Besides conflating the concepts of name and value, it's also impractical. You wouldn't, for instance, be able to print something like "You entered a = 23", because you no longer know both the letter and the entered value.
This is redundant
let = Float(gets.chomp) sum += let.to_f
You create a
Float
from the input. And then you callto_f
on that float - but it's already a float. Either useFloat(...)
to get a number, or use the.to_f
on a string. The latter is the conventional approach, but the former will complain if the input isn't numeric, so that's probably the one you'll want to use here:sum += Float(gets.chomp)
And we've avoided overwriting
let
too.But speaking of code complaining: You might want to be more specific in your exception handling. Your current
rescue
block doesn't distinguish between the sorts of errors that might arise. It's one thing to complain to the user if their input is bad, but what if the error is about something else? For instance, if I run your code in my editor, it'll run without an interactive terminal. Sogets
immediately returnsnil
, which causes aNoMethodError
sincechomp
isn't a method onnil
. And sincegets
returns immediately, the script goes into a retry-loop, spewing "Something went wrong. Try again." as fast as it can until I kill it.Obviously, this isn't how the script is meant to be run, but it goes to show how telling the user to "try again" isn't always a solution. Your code just assumes it's always the user's fault, and if it tries enough times, the problem will get fixed. But the error could also be a mistake in the code, like writing
get
instead ofgets
- that'd also cause the script to loop indefinitely, even though there's nothing the user can do to salvage things.
Now, for alternative approaches: While define_method
is a really neat Ruby feature, it's not called for in this case. But a generalized method for getting a float as user input would be useful. Then you can use that for the rest of the code. For instance, the script could be:
def read_float(prompt)
puts prompt
Float(gets.chomp)
rescue ArgumentError # only rescue this type of error
puts "Please enter a number"
retry
end
puts %w(a b).reduce(0) { |sum, var| sum + read_float("Enter #{var}") }
Yes, I'm being a little clever with the reduce
and everything. A more straight-forward and instantly readable way would be to say:
a = read_float("Enter a")
b = read_float("Enter b")
puts a + b
which is about as to-the-point of "A + B = C" as you can get.
You may already know this, but just to be sure, since I didn't see this answer yet.
def add(a,b)
a + b
end
add(4,5) => 9
The simplest way I can think of to define add
(I realize you were probably just trying out things!)
Your indentation is off. The first line should have no indentation, and the begin
should line up with the end
etc. That code looks more like this:
puts "<<<-----ADDITION----->>>"
puts "<<<------A+B=C------->>>"
letters = %i(a b)
sum = 0
letters.each do |let|
define_method(let) do
begin
puts "Enter #{let}"
let = Float(gets.chomp)
sum = sum + let.to_f
rescue
puts "Something went wrong. Try again."
retry
end
end
end
letters.each do |let|
send(let)
end
puts sum
Also that Float
and to_f
is redundant.
Next, you have a variable issue. let
is a Symbol
that is used for defining a variable, until this line:
let = Float(gets.chomp)
This reassigns let
, and reuses it as a Float
, which would be fine, except for the fact that the variable was defied outside the method. this means that it will keep its value between runs. So if you run the a
method a second time, it would output something like:
Enter 3.0
Which is not what you want. what you want it to do is reassign the value of the a
in a+b=c
. The code would be something like this (code modification is marked with comments):
letters.each do |let|
val = 0 #define outside the method to make it persist.
define_method(let) do
begin
old_val = val #store the old value later
puts "Enter #{let}"
val = Float(gets.chomp)
sum = sum + val - old_val #subtract the old value away, to replace it
rescue
puts "Something went wrong. Try again."
retry
end
end
end
With this code you can run the a
or b
methods again and it will modify sum
properly.