-
Notifications
You must be signed in to change notification settings - Fork 51
Branches - Xinran #28
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.
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
base: master
Are you sure you want to change the base?
Conversation
CalculatorWhat We're Looking For
Well done on this project, Xinran! I think that your code is very organized, readable, and logical. You have a great solution for calculator, and you did a lot of nice, clever things. Your conditionals, loops, and methods look great! If you had more time on this project, I may suggest looking over your code to see that there are more opportunities for methods to be pulled out. For example, getting the user input for the two numbers is some repeated code. I've also left some comments with other small suggestions. Overall, your code looks great! I really love the methods you pulled out and how you organized the code and your use of the cool array things, like |
|
|
||
| puts "What is your first number?" | ||
| first_number = gets.chomp | ||
| while first_number.to_i.to_s != first_number && first_number.to_f.to_s != first_number # verify if the input is valid, both integer and float should be valid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice strategy!
| perform_calculation_two_numbers(operator, first_number, second_number) | ||
| end | ||
|
|
||
| def calculator_methods_display |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I LOVE that you created a separate method used to display information! Keep doing things like this!
| perform_calculation_two_numbers(operator, first_number, second_number) | ||
| end | ||
|
|
||
| def calculator_methods_display |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer my methods to have names that are like actions (so, method names that start with verbs). I may suggest renaming this method to display_calculator_methods or show_calculator_methods, just like how you named a method perform_calculation_two_numbers
| puts "Your calculation is #{num1}^#{num2} = " + "#{exponent_display}" + " = #{result}" | ||
| elsif num2 < 0 | ||
| exponent_arr = [] # create an array to store num2 num1s | ||
| (-num2).to_i.times do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, you deal with the case such that if num2 is negative, you say to take the -num2 of it. Instead of making num2 negative, could you consider taking the absolute value of num2? You can do that in Ruby using .abs, so here, consider:
(num2.abs).to_i.times do
exponent_arr << num1
end
Calculator
Congratulations! You're submitting your assignment.
Comprehension Questions