Skip to content

Branches - Michaela #35

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

michaela260
Copy link

Calculator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
What went well in your code style, such as indentation, spacing, variable names, readability, etc.? What was lacking? I think that my indentation and spacing worked well and that the code is generally readable. I am still hoping to improve my comments to know which are necessary and unnecessary for user readability and best practices. Also, some of my comments are a bit long.
How did your code keep track of user input? The code stored user input in the specified variables chosen_operator and chosen_number. New values were assigned to these variables when the user submitted updated input (after an invalid input, for example).
How did your code determine what operation to perform? The program determined which operation to perform by using the "send" method to interpret a string such as "+" or "-" as a method rather than string. The code handled written operators (i.e. "plus" or "minus") by linking each of these to their corresponding operator (+, -, etc.) in a key-value pair within the hash "operators."
What opportunities exist to create small methods for this project? You could define a method for each operator (multiply divide, etc.) and then use case/when to call the given method for each set of input string operators. You could also create methods for each type of error request, and a method to display the valid user input options (valid operators).
In the next project, what would you change about your process? What would you keep doing? In the next project, I would complete the basic requirements before delving deep into rabbit holes of multiple possible solutions. I think I would spend a bit less time trying to make multiple solutions work and instead do that at the end if I have extra time. I would continue to experiment with using new methods and types of code that I have not tried before because it was good practice.

@michaela260 michaela260 changed the title Create calculator.rb Branches - Michaela Aug 7, 2019
@tildeee
Copy link
Collaborator

tildeee commented Aug 9, 2019

Calculator

What We're Looking For

Feature Feedback
Readable code with consistent indentation x
Practices using variables appropriately x
Practices using conditionals appropriately x
If any, practices iteration appropriately x
If any, practices using custom methods appropriately x
Takes in two numbers and an operator and can perform addition x
Takes in two numbers and an operator and can perform subtraction x
The program handles divide when attempting to divide by zero x

Fantastic work on this project, Michaela! Your solutions are so well-organized, clever, logical, and readable. I really really really like the specific methods you pulled out for this assignment.

I have a few comments and suggestions on how I might push this assignment for more refactoring if you had more time on it-- mostly, some ways to refactor those methods.

However, overall this was a great project submission. Great work!


# Check user-input operator for validity and store valid operator
chosen_operator = gets.chomp.downcase
chosen_operator = error_message_operators(chosen_operator, operators)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error_message_operators method is so so so clever. I like it a lot! Keep doing things like this! It may be interesting to rename this method to something like get_operator, and refactor the code so that you only pass in operators (instead of chosen_operator and operators) and also get the initial user operator in the method, too. For example, consider:

def get_operator(valid_input)
  input = gets.chomp.downcase
  until valid_input.keys.include? input.to_sym
    puts "Oops! Your input was invalid. Your options are:"
    print_operators
    input = gets.chomp
  end
  return input
end


# Check user-input numbers for validity and store valid numbers
chosen_number_1 = Float(gets) rescue nil
chosen_number_1 = error_message_numbers(chosen_number_1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my suggestion above, it may be interesting to refactor to a name like get_number, and that method could change to

def error_message_numbers
  input = Float(gets) rescue nil
  while input == nil
    puts "Oops! Your input was invalid. Please enter a number: "
    input = Float(gets) rescue nil
  end
  return input
end

# Call the selected operator method with the user's selected number arguments
# Store the resulting value as the answer
# Note: operators are methods (send reads the given string as a method)
answer = chosen_number_1.send(operators[:"#{chosen_operator}"], chosen_number_2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this solution RULES! In practicality, I hope that you are never in a work situation where you will realistically need to use .send, because that's a dangerous method! But for this specific case, it ends up being perfect. Well done, and nice work combining it with your operators lookup hash.

puts "5. Modulo (%)"
puts "6. Exponent (^)"
print "\nPlease choose an operator (name or symbol): "
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this method!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants