-
Notifications
You must be signed in to change notification settings - Fork 52
Branches - Macaria #32
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
Macaria, this project submission is wonderful! It's very well-organized, thoughtful, logical, and readable. I'm particularly excited about your I have a few comments and suggestions on a few places of refactoring if you had more time on this project. Well done on your solution overall! Great work! |
@@ -0,0 +1,66 @@ | |||
operand = () |
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.
Do you need this variable to be defined outside of the method?
@@ -0,0 +1,66 @@ | |||
operand = () |
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.
As a separate note, if operand was something we needed, instead of ()
, it may be good to give operand
a default value, or set it to nil
. Maybe something like operand = 0
or operand = nil
, just like you did with operator_select
Exponify(**) | ||
|
||
Please select an operator" | ||
operator = gets.chomp.to_s.downcase |
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.
Because you mentioned that you would like to make more methods, and also because your get_operand
method was really nice, I may suggest making a get_operator
method. Even if a method to get the operator is used once, the benefits of the organization can be worth it!
puts "What is your second number?" | ||
operand2 = get_operand() | ||
|
||
# Validate that the user is not trying to divide by 0 |
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.
Good style and frequency of comments throughout the whole submission!
|
||
# Output equation and solution | ||
puts "Let's do this math!" | ||
puts "#{operand1} #{operator_select} #{operand2} = #{operand1.to_f.public_send(operator_select, operand2.to_f)}" |
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