Skip to content

Naheed - Calculator - Edges #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Naheed - Calculator - Edges #32

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions calculator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
puts "Welcome to your calculator. Choose from the following operations: \n 1. add (+) \n 2. subtract (-) \n 3. multiply (*) \n 4. divide (/) \n Please type the name or symbol for the operator you would like to use"

chosen_operator = gets.chomp!

#gets numbers from user
puts "First number:"
first_number = gets.chomp.to_f
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've written almost exactly the same code twice to get the two numbers from the user. As is it's only two lines, so it's not so bad, but it makes it difficult to expand that logic. For example, you might want to run this code in a loop until the user enters a valid number. Doing this in a method would mean you only have to write that code once.

puts "Second number:"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watch your indentation. I don't think these lines should be indented at all.

second_number = gets.chomp.to_f

#checking if number is integer by dividing it by itself
if first_number / first_number == 1 && second_number / second_number == 1

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a creative way to address this problem. Unfortunately it's got one big issue: it doesn't work for 0!

if chosen_operator == "add" || chosen_operator == "+"
puts "#{first_number} + #{second_number} = #{first_number + second_number}"
elsif chosen_operator == "subtract" || chosen_operator == "-"
puts "#{first_number} - #{second_number} = #{first_number - second_number}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't repeated, but I think it would still increase readability to wrap this if/elsif section in a method. That would clearly delineate where it starts and ends, and make it explicit what data it needs to work. The method signature might be something like perform_calculation(chosen_operator, first_number, second_number).

elsif chosen_operator == "multiply" || chosen_operator == "*"
puts "#{first_number} * #{second_number} = #{first_number * second_number}"
elsif chosen_operator == "divide" || chosen_operator == "/"
puts "#{first_number} / #{second_number} = #{first_number / second_number}"
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have an else clause here so that when the user inputs an invalid operator, you can give them a helpful message about what went wrong. As is your program fails silently.


#if number wasn't an integer, will display option to re-input
elsif puts "Sorry, that wasn't a number. Try again."
end