Brandy Austin - Calculator - Octos#19
Conversation
CalculatorWhat We're Looking For
Great job overall! I have a couple of inline comments below that you should check out, but in general I am quite happy with this submission. Keep up the hard work! |
| # perform addition | ||
| def addition(num_1, num_2) | ||
| num_1 + num_2 | ||
| return num_1 + num_2 |
There was a problem hiding this comment.
These two lines are redundant - you should be able to get away with only the second.
| # format_userinput_not_including_division formats the numbers, but doesn't perform the calculation | ||
| def format_userinput_for_division(operator, valid_operators, num_1, num_2, user_input) | ||
| if num_1 =~ /^(\d)+$/ && num_2 =~ /^(\d)+$/ | ||
| unless divide_by_zero(num_2.to_i) |
There was a problem hiding this comment.
Doesn't the user_input include num_1, num_2 and the operator?
| # request operation, num_1, and num_2 from user then store in hash | ||
| def request_operations_numbers(user_input, valid_operators) | ||
| puts "What operation would you like to perform?" | ||
| operation = gets.chomp |
There was a problem hiding this comment.
We talked about this a little in person, but I'll recap here:
I like that you've broken this out as a separate method, but passing all the user input around as a hash is a little unwieldy. Another possible approach is to have one method to read a number, which is called twice, and another to read the operator.
| # only formats the numbers, does not perform the actual calculation | ||
| def format_userinput_not_including_division(valid_operators, num_1, num_2, user_input) | ||
| if num_1 =~ /^(\d)*\.(\d)+$/ | ||
| num_1 = num_1.to_f |
There was a problem hiding this comment.
You have the same code repeated twice in this function, the only difference being num_1 or num_2. Is there a way you could consolidate this?
Maybe change it to be a method that takes one number in string form, and returns that number as an int or float as appropriate.
| format_userinput_not_including_division(valid_operators, user_input[:num_1], user_input[:num_2], user_input) | ||
| result = addition(user_input[:num_1], user_input[:num_2]) | ||
| result = format_if_result_is_zero(result) | ||
| puts "#{user_input[:num_1]} + #{user_input[:num_2]} = #{result}" |
There was a problem hiding this comment.
These last two lines (calling format_if_result_is_zero and putsing) are almost the same for every operation. Could you eliminate this redundancy somehow?
brandyaustinseattle
changed the title
2 participants
Calculator
Congratulations! You're submitting your assignment.
Comprehension Questions