-
Notifications
You must be signed in to change notification settings - Fork 51
Branches - Kelsey #37
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
Great work on this project, Kelsey! The flavor of this project submission is ideal, honestly. Also, your code is great! The code is logical, readable, and well-organized. You did a lot of really clever things! I have a couple comments that are mostly FYIs, with some minor ideas/suggestions. That being said, the submission looks great; great work! |
|
|
||
| operands.each do |operand| | ||
| puts "#{operand[:id]}. #{operand[:name]} #{operand[:symbol]}" | ||
| end |
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.
Fancy!
| puts "YOU HAVE CHOSEN EXPONENTIATION." | ||
| math_operand = :** | ||
| when "6", "MOD", "%" | ||
| puts "YOU HAVE CHOSEN MODULO-IFICATION. \n(CALCU-BOT DOES NOT KNOW THE VERB TENSE FOR A MODULO.)" |
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.
😂 😂 😂
| def float_checker | ||
| self.to_f.to_s == self | ||
| end | ||
| end |
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.
THIS IS COOL! And fancy! You are doing a thing which is adding a new method to the String class, which is what the user input it. We will definitely get into these details in a few weeks.
If you were curious about how to do this without working with the String class, I would likely make the methods integer_checker and float_checker not part of String, and define that it takes in a number, and replacing the self with that number, so:
def integer_checker(num)
return num.to_i.to_s == num
endNot a mandatory refactoring, but just wanted to give you this FYI!
|
|
||
|
|
||
| # returns error message until user inputs an integer or float | ||
| def class_check(num) |
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.
Minor suggestion: I tend to really like having my method names read more like verbs. What is this method responsible for? It's used for checking what kind of number it is. There are a lot of different good names for this, but maybe I would name this check_valid_number_type or something like that
| puts "CALCU-BOT CANNOT DIVIDE BY ZERO. ENTER ANOTHER NUMBER." | ||
| second_num = gets.chomp | ||
| end | ||
| end |
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.
Nicely done!
| puts "CALCU-BOT HAS CALCULATED THE CALCULATION." | ||
|
|
||
| def operate_numbers(symbol, num1, num2) | ||
| result = num1.public_send(symbol, num2) |
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.
Fantastic solution with using public_send! It ends up being a great solution for this assignment. In future projects and assignments, it'll be really dangerous and weird for us to use public_send, so I'm just calling out right now that I don't expect to use this basically ever again. :P
Calculator
Congratulations! You're submitting your assignment.
Comprehension Questions