0

This is my first foray into computer programming. I have chosen to learn Ruby, and I am enjoying it quite a bit. However, I am a little confused as to why the answer will not output properly in this bit of code.

def addition_function
    puts "Which numbers would you like to add?"
    @n1 = gets.chomp
    @n2 = gets.chomp
    @n1 + @n2 == @answer
    puts "The sum is... #{@answer}"
end

def subtraction_function
    puts "Which numbers would you like to subtract?"
    @n1 = gets.chomp.to_i
    @n2 = gets.chomp.to_i
    @n1 - @n2 == @answer
    puts "The answer is... #{@answer}"
end

def multiplication_function
puts "Which numbers would you like to multiply?"
    @n1 = gets.chomp
    @n2 = gets.chomp
    @n1 * @n2 == @answer
    puts "The answer is... #{@answer}"
end

puts "Would you like to [add], [multiply], or [subtract]?"
response = gets.chomp
if response == "add" then
    addition_function
end
if response == "subtract" then
    subtraction_function
end
if response == "multiply" then
    multiplication_function
end

I know this is probably horrible code... but could someone help steer me in the right direction?

orde
  • 5,233
  • 6
  • 31
  • 33
RogueWav
  • 21
  • 1
  • 1
  • 3
  • 1
    You don't need to suffix your methods with `_function`. It is preceded with `def`, which is how you create methods. It is unnecessary. – Justin Wood Dec 31 '13 at 02:07
  • 1
    The code is not properly indented, you DON'T need `then` in the `if` clause, and you should be calling to_i (converting string to integer) if you want arithmetic operations – bjhaid Dec 31 '13 at 02:07

11 Answers11

6

Consider this code:

def get_int_values
  [gets, gets].map{ |s| s.chomp.to_i }
end

puts "Would you like to [add], [multiply], or [subtract]?"
response = gets.chomp

case response.downcase
when 'add'
  puts "Which numbers would you like to add?"
  operator = :+

when 'subtract'
  puts "Which numbers would you like to subtract?"
  operator = :-

when 'multiply'
  puts "Which numbers would you like to multiply?"
  operator = :*

end

answer = get_int_values.inject(operator)
puts "The answer is... #{ answer }"

The idea is to follow the "DRY" principle: "DRY" means "Don't Repeat Yourself", which the vast majority of the time, is a really good thing.

To help avoid typing mistakes I'd recommend doing something like:

puts "Would you like to [a]dd, [m]ultiply, or [s]ubtract?"
response = gets.chomp

case response[0].downcase

then change the when clauses to match the first letter of the desired operation.

Which will work unless response is empty. You can figure out how to handle that.


another way to obtain answer, once operator is determined, is answer = gets.to_i.send(operator, gets.to_i)

That's true, but here's why I refactored the code the way I did: If, for some reason, there was a need to operate on more than two values, only one thing has to be changed:

[gets, gets].map{ |s| s.chomp.to_i }

could become:

[gets, gets, gets].map{ |s| s.chomp.to_i }

Or, better, could be transformed to something like:

def get_int_values(n)
  n.times.map { gets.chomp.to_i }
end

Nothing else will have to change except to find out how many values are needed.

Now, to do it all right would require different text to alert the user that multiple values are expected, but that's easily done by letting letting the user say how many they want to enter, and then prompting for each gets:

def get_int_values(n)
  n.times.map.with_index { |n|
    print "Enter value ##{ 1 + n }: "
    gets.chomp.to_i 
  }
end

puts "Would you like to [add], [multiply], or [subtract]?"
response = gets.chomp

puts "How many values?"
num_of_values = gets.to_i

case response.downcase
when 'add'
  puts "Which numbers would you like to add?"
  operator = :+

when 'subtract'
  puts "Which numbers would you like to subtract?"
  operator = :-

when 'multiply'
  puts "Which numbers would you like to multiply?"
  operator = :*

end

answer = get_int_values(num_of_values).inject(operator)
puts "The answer is... #{ answer }"

inject can scale up easily because it doesn't presuppose knowledge about the number of values being operated on.


I think with_index in n.times.map.with_index is an artifact you forgot to delete.

It was deliberate but I like this better:

def get_int_values(n)
  1.upto(n).map { |n|
    print "Enter value ##{ n }: "
    gets.chomp.to_i
  }
end
the Tin Man
  • 158,662
  • 42
  • 215
  • 303
  • Good advice, the. Rogue, another way to obtain `answer`, once `operator` is determined, is `answer = gets.to_i.send(operator, gets.to_i)`. Understanding `send` should be near the top of your list for learning Ruby. – Cary Swoveland Dec 31 '13 at 06:03
  • Thank you for this feedback. It amazes me how much simpler and elegant this code is to what I created... – RogueWav Dec 31 '13 at 14:39
  • @CarySwoveland, yes, `send` is a possibility, but it doesn't scale well if the problem changes to `n` number of values being processed. See the edit to the answer. – the Tin Man Dec 31 '13 at 17:52
  • @RogueWav, Ruby is a great little language. It's very expressive and makes it easy to write in a number of different styles; I've written in a lot of different languages over the years, and Ruby amazes me with its elegance and how nicely it can break things down. – the Tin Man Dec 31 '13 at 17:54
  • I think `with_index` in `n.times.map.with_index` is an artifact you forgot to delete. Reply not reqd. Will delete this comment. – Cary Swoveland Dec 31 '13 at 18:15
  • Or, instead of the `case` statement, `op = {"add" => :+, "subtract" => :-, "multiply" => :*}`, `puts "Which numbers do you want to #{response}"; answer = get_int_values(num_of_values).inject(op[response])`. – Cary Swoveland Dec 31 '13 at 18:37
  • @CarySwoveland, `with_index` is there deliberately, to allow an incrementing counter used to provide the prompt. If the OP were to need 100 values, it'd be a very error-prone process with a high likelihood of missing entries without some sort of place-holder. I modified the code to something I like better though. – the Tin Man Dec 31 '13 at 21:31
  • What I meant was that you can map on the iterator, just as you can with `0.upto(n-1)`; e.g., `3.times.map {|i| i+1} # => [1,2,3]`. – Cary Swoveland Dec 31 '13 at 23:08
1

Your assignments are on the wrong side of the statement. You should have answer = n1 * n2, which is not the same as answer == n1 * n2 (this is a check for equality, using ==). The expression always goes on the right, and the variable the result is assigned to goes on the left -- this is pretty much universal, but not necessarily intuitive coming from algebra.

Also: using an @ prior to a variable name differentiates it as an instance variable, or member, of a class. From what you've shown here you don't need to include those, just normally scoped variables are required for this use. Check out this question for more on that part.

Community
  • 1
  • 1
Kyle Travis
  • 249
  • 1
  • 4
1

The "@" sigil is used to indicate a class instance variable, you have no class so don't use it.

@n1 + @n2 == @answer

Is a boolean expression evaluating whether @n1 + @n2 is equal to @answer.

It will evaluate to true or false.... but you don't make use of the answer.

What you want is ...

answer = n1 + n2

I strongly recommend you always run Ruby with the -w option. It will save you much much heartache.

Please indent your "end"'s to match your "def" (or "if").

You repeat n1 = gets.chomp.to_i all over the place, do it once and pass the answers as a parameter...

response = gets.chomp
n1 = gets.chomp.to_i
n2 = gets.chomp.to_i

if response == "add" then
   addition_function( n1, n2)
elsif...
John Carter
  • 460
  • 3
  • 10
  • 1
    Just "instance variable" ; "class instance variable" is [something else](http://rubymonk.com/learning/books/4-ruby-primer-ascent/chapters/45-more-classes/lessons/113-class-variables). – Cary Swoveland Dec 31 '13 at 04:01
1

A few suggestions not mentioned by others:

  • Shorten your method (not "function") names and use verbs (e.g., add instead of addition_method).

  • As well as using local variables rather than instance variables (mentioned by others), eliminate them where you can. For example, you could simplify

.

   def add
     puts "Which numbers would you like to add?"
     n1 = gets.to_i
     n2 = gets.to_i
     answer = n1 + n2
     puts "The sum is... #{answer}"
   end

to

   def add
     puts "Which numbers would you like to add?"
     puts "The sum is... #{gets.to_i + gets.to_i}"
   end
  • Notice I've used the Ruby convention of indenting two spaces.

  • You don't need chomp here (though it does no harm), because "123followed by \n or any other non-digits".to_i => 123.

  • A case statement would work well at the end (and let's loop until the user chooses to quit):

.

   loop do
   puts "Would you like to [add], [multiply], [subtract] or [quit]?"
     case gets.chomp
     when "add"
       add
     when "subtract"
       subtract
     when "multiply"
       multiply
     when "quit"
       break
   end

or just

   def quit() break end

   loop do
     puts "Would you like to [add], [multiply], [subtract] or [quit]?"
     send(gets.chomp)
   end
  • Here we do need chomp. You could replace loop do with while true do or use other equivalent constructs.
Cary Swoveland
  • 106,649
  • 6
  • 63
  • 100
1

class Calculator

def Calc

   puts"==well come to mobiloitte calculator=="

puts "enter the first operand:" 
@op1 = gets.chomp
return if @op1=="q"
 @o1=@op1.to_i
puts "entre the second operand:"
 @op2 = gets.chomp
return if @op2=="q"
 @o2=@op2.to_i

strong text puts "enter any one operator of your choice (add,sub,mul,div,mod)" operator = gets.chomp

case  operator
 when 'add' then @s=@o1+@o2 ; puts "\n #@o1 + #@o2 =#@s"
 when 'sub' then @t=@o1-@o2 ; puts "\n #@o1 - #@o2 =#@t"
 when 'mul' then @l=@o1*@o2 ; puts "\n #@o1 * #@o2 =#@l"
 when 'div' then @r=@o1/@o2 ; puts "\n #@o1 \ #@o2 =#@r"
 when 'md' then  @d=@o1%@o2 ; puts "\n #@o1 % #@o2 =#@d"

    else  
     puts"invalide input"

end end end obj= Calculator.new $f=obj.Calc

roshan
  • 11
  • 1
0

You are using @n1 + @n2 == @answer to try and set the answer. What you want to do is @answer = @n1 + @n2.

= is assignment, == is a comparison operator.

Also, you will need to @n1 = gets.chomp.to_i. This will convert your input to an integer from a string. Do that with @n2 as well.

You also do not need to use the @ before each of your variables. That should only be used when you are dealing with classes, which you do not appear to be doing.

Justin Wood
  • 9,941
  • 2
  • 33
  • 46
0
print "enter number 1 : "
n1 = gets.chomp.to_f
print "enter number 2 : "
n2 = gets.chomp.to_f
print "enter operator: "
op = gets.chomp
if op == '+'
  puts "#{n1} + #{n2} = #{n1 + n2}"
elsif op == '-'
  puts "#{n1} - #{n2} = #{n1 - n2}"
elsif op == '*'
  puts "#{n1} * #{n2} = #{n1 * n2}"
elsif op == '/'
  puts "#{n1} / #{n2} = #{n1 / n2}"
end
  • Could you explain your code in more detail? This is not very expressive nor easy to understand if you are new to programming as the question states. – while Oct 03 '14 at 12:23
0
puts "Would you like to 
    0 ---- [exit],  
    1 ---- [add], 
    2 ---- [subtract], 
    3 ---- [multiply], 
    4 ---- [divide]"
response = gets.chomp

case response.downcase
when '1'
def addition_function
            puts "Which numbers would you like to add?"
            n1 = gets.to_i
            n2 = gets.to_i
            answer = n1 + n2
            puts "The sum is... #{n1} + #{n2} = #{answer}"
end
addition_function()


#Subtract
when '2'
def subtraction_function
            puts "Which numbers would you like to subtact?"
            n1 = gets.to_i
            n2 = gets.to_i
            answer = n1 - n2
            puts "The subtraction is... #{n1} - #{n2} = #{answer}"
end
subtraction_function()
#Multiply
when '3'
def multiplication_function
            puts "Which numbers would you like to multiply?"
        n1 = gets.to_i
            n2 = gets.to_i
            answer = n1 * n2
            puts "The multiplication is... #{n1} * #{n2} = #{answer}"
end
multiplication_function()


#Division
when '4'
    def division_function
            puts "Which numbers would you like to divide?"
            n1 = gets.to_i
            n2 = gets.to_i
            answer = n1 / n2
            puts "The division is... #{n1} / #{n2} = #{answer}"
    end
division_function()

else '0'
    puts "Exit! Thank You for using us!"
end
Pang
  • 9,564
  • 146
  • 81
  • 122
0
#ruby script to do the calculator
puts " enter the number1"
in1=gets.to_i
puts " enter the number2"
in2=gets.to_i
puts "enter the operator"
op=gets.chomp
case op
when '+'
    plus=in1+in2
    puts "#{in1+in2}"
    #puts "#{plus}"

when '-'
    min=in1-in2
    puts "#{min}"
when '*'
    mul= in1*in2
    puts "#{mul}"
when '/'
    div=in1/in2
    puts "#{div}"
else
    puts "invalid operator"
end
Anoob K Bava
  • 598
  • 7
  • 19
0
begin
  puts 'First number:'
  a = $stdin.gets.chomp.to_i

  puts 'Second number:'
  b = $stdin.gets.chomp.to_i

  operation = nil
  unless ['+', '-', '*', '/', '**'].include?(operation)
    puts 'Choose operation:  (+ - * /):'
    operation = $stdin.gets.chomp
  end

  result = nil
  success = false

  case operation
  when '+'
    result = (a + b).to_s
  when '-'
    result = (a - b).to_s
  when '*'
    result = (a * b).to_s
  when '/'
    result = (a / b).to_s
  when '**'
    result = (a**b).to_s
  else
    puts 'There is not such kind of operation'
  end
  success = true
  puts "Результат: #{result}"
rescue ZeroDivisionError => e
  puts "You tried to devide number by zero! Error: #{e.message}"
end

if success
  puts "\nSuccess!"
else
  puts "\nSomething goes wrong :("
end
0
 puts ("plz enter a number :")
 num1 = gets.chomp.to_f
 puts ("plz enter a another number")
 num2 = gets.chomp.to_f
 puts ("plz enter the operation + , - , x , / ")
 opp = gets.chomp 
 if opp == "+"
 puts (num1 + num2)
 elsif opp == "-"
 puts (num1 - num2)
 elsif opp == "x"
 puts (num1 * num2)
 elsif opp == "/"
 puts (num1 / num2)
 else puts ("try again :|")
 end 
  • While this answer has correct code - please add some explanation. The original poster probably doesn't understand that his missing 'to_f' and other conversions are the reason the code isn't functioning, nor what his other problems are. – TreyE Sep 17 '22 at 15:50