Skip to content

Semret - Calculator - Edges#31

Open
snicodimos wants to merge 1 commit intoAda-C10:masterfrom
snicodimos:master
Open

Semret - Calculator - Edges#31
snicodimos wants to merge 1 commit intoAda-C10:masterfrom
snicodimos:master

Conversation

@snicodimos
Copy link

Calculator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe how you stored user input in your program? using a variable
How did you determine what operation to perform? using an if/else statement to verify user input for operations
Do you feel like you used consistent indentation throughout your code? Yes - tried to incorporate Ruby style as much as possible
If you had more time, what would you have added to or changed about the program? I would have used an array to store operations, I would have made a method to check for input validation for numerical inputs, I would make sure my program allows input in both float and integer types, I would make my code a bit DRYer

@droberts-sea
Copy link

Calculator

What We're Looking For

Feature Feedback
Takes in two numbers and an operator and performs the mathematical operation. yes
Readable code with consistent indentation. mostly - see inline

Good job overall! This code works well, and you've done a good job of using whitespace to break up the code and make it more readable. There are a few places where you could have used methods to DRY up the code or make it more readable, which I've tried to call out below, but in general I am quite happy with this submission. Keep up the hard work!

# Ask user for numerical value input and validate input to be an integer
puts "Please input your numbers"
puts "First number: "
first_num = gets.chomp

Choose a reason for hiding this comment

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

Watch your indentation here.

puts "Second number: "
second_num = gets.chomp
until !((second_num !='0') && (second_num.to_i.to_s != second_num))
puts "Please enter a valid number"

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 here twice, to get the first number and the second number. Could you DRY that up by putting this logic in a method?

if operation == "add" || operation == "+"
puts "#{first_num} + #{second_num} = #{addition(first_num, second_num)}"
elsif operation == "subtract" || operation == "-"
puts "#{first_num} - #{second_num} = #{subtraction(first_num, second_num)}"

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/elseif 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(operation, first_num, second_num).

attempt = 0
while !(operation == "add" || operation == "+" || operation == "subtract" || operation == "-" || operation == "multiply" || operation == "*" || operation == "divide" || operation == "/")
if attempt < 3
puts "Oops your input was not recognized. Please try again "

Choose a reason for hiding this comment

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

This part where you get an operation from the user is another place where it would be good for readability to wrap it in a method.

if second_num == 0 || second_num == nil
puts "You can not divide any number by zero or nil."
else
puts "#{first_num} / #{second_num} = #{division(first_num, second_num)}"

Choose a reason for hiding this comment

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

Why not put this logic to detect zero division inside the division method?

# additions
def addition(num1, num2)
return num1 + num2
end

Choose a reason for hiding this comment

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

I like that you've made each of these a separate method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants