-
Notifications
You must be signed in to change notification settings - Fork 51
Branches - Monick #33
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
Fantastic work on this Monick! Your code is so clean, logical, and readable, and a lot of the things you did were particularly clever and/or elegant. I'm really happy how you practiced making methods and using the If you had more time on this, I would LOVE to see you pull the logic of getting user input and checking its validity into a method. The code that you wrote for getting both numeric input is the same! I've added a few other comments, some of them about the Overall, this project was GREAT! Great work! |
|
|
||
| #method for checking if value is numeric | ||
| def is_number? string | ||
| true if Float(string) rescue false |
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! This method you defined is sooo useful and so elegant! In general, I may suggest an explicit return with return true if Float(string) rescue false, even though it ruins how elegant it looks. SORRYYY
| #check to see if user is trying to divide by 0 | ||
| if num2 == 0 | ||
| #if num2 is 0, then tell user, not possible. That's rude and exit program. | ||
| puts "Looks like you're trying to divide by 0. You know that's mean. BYE BYE FELICIA!" |
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.
🙈
| puts "Here is a list of types of math" | ||
| operations_list.each do |key, value| | ||
| puts "#{key} (#{value})" | ||
| 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 so fancy and it makes me happy. Maybe consider renaming key and value to operation and symbol?
| puts "What kind of math do you wanna do? " | ||
| user_operator = gets.chomp | ||
| #call on check_operator method to check if user entered a valid operation | ||
| if variations_list.include?(user_operator) == false |
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.
Another way to write this is if !variations_list.include?(user_operator), because the ! negates it
| puts "Cool. Now, what's the first number you wanna calculate?" | ||
| user_num1 = gets.chomp | ||
| #check to see if input is valid (a number to use for computation) | ||
| if is_number?(user_num1) == false |
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.
Another way to write this is if !is_number?(user_num1)
| #create a calculator program that asks for user input and runs file from command line | ||
|
|
||
| #define global scope variables | ||
| results = nil |
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.
Totally a small thing, but you named this variable results, when it never will be an array, it will always be one result. Consider renaming this to result
Calculator
Congratulations! You're submitting your assignment.
Comprehension Questions