Skip to content
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

Caroline Nardi - Calculator - Octos #31

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

Conversation

cmn53
Copy link

@cmn53 cmn53 commented Feb 7, 2018

Calculator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe how you stored user input in your program? I initially stored the user input as a string, then passed it to the parse method to split the string into an array with three elements. The first and third elements of the string stored the two numbers for the calculation, and the second element stored the operator.
How did you determine what operation to perform? I used a case statement where the conditions were the various possible operators.
Do you feel like you used consistent indentation throughout your code? I tried to... I'm still figuring this out. I'm particularly not clear on whether comments should also be indented with the line they are referring to.
If you had more time, what would you have added to or changed about the program? I didn't get to the optional parenthesis feature, but I think the way I set up the parse method would lend itself to being able to do this without too many changes to the structure of the rest of the program. If I would have had more time, I would have figured out how to strip out commas and created a more robust validation method for the input. In particular, I could have spent more time creating a better regex to validate the numbers -- what I have in there right now would reject anything with a comma, without a number before the decimal point, etc. There is probably also a better way to determine if a string should be a float or an integer than just looking for a decimal point. Finally, I would like to figure out why inputting a negative sign generated a preceding empty string in the parsed array.... I suspect it might have something to do with "-" not being a special character that needs to be escaped, but that's just a guess.

@CheezItMan
Copy link

Calculator

What We're Looking For

Feature Feedback
Takes in two numbers and an operator and performs the mathematical operation. Check
Readable code with consistent indentation. One minor mistake, see my inline comments
Summary Nicely done, You hit the primary requirements. See my notes on your code inline. See my notes inline for specific comments. Well done!

def parse(input)
# Removes white space, converts string into an array split by the possible operators (but retaining the operators).
# Removes empty strings from the array. I'm actually not sure where these are coming from, but I noticed them when the user enters a negative number.
parsed_input = input.gsub(/\s+/,"").split(/(\+)|(-)|(\*)|(\/)|(\^)|(%)|(add)|(subtract)|(multiply)|(divide)|(exponent)|(modulo)/).reject { |x| x.empty? }

Choose a reason for hiding this comment

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

Nice regular expression to split the input string.


# Informs user of the limits of the calculator, the possible operators, and prompts user for their calculation.
puts "Welcome to my (very limited) calculator. This calculator can perform basic arithmetic operations between two numbers."
puts "\nThe accepted operators are: +, -, *, /, ^, or %, or the words add, subtract, multiply, divide, exponent, or modulo."

Choose a reason for hiding this comment

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

I would suggest an example of valid input. At first I was entering just the operator (-,+,/,*, etc).

puts "Hmm. It doesn't look like the operation you've entered is valid for this calculator."
# If the array length is less than 3, the user likely didn't enter enough information,
# or didn't enter it in a manner that the parser could handle. Returns nil.
elsif (parsed_array[0] =~ /^[-+]?\d+(\.\d+)?$/).nil? || (parsed_array[2] =~ /^[-+]?\d+(\.\d+)?$/).nil?

Choose a reason for hiding this comment

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

Nice regular expression, I would suggest to use /^[-+]?\d*(\.\d+)?$/ instead of what you have there. Yours requires at least 1 digit to the left of the decimal so .25 wouldn't work.

parsed_input[3] = parsed_input[2..3].join
parsed_input.slice!(2)
end
return parsed_input

Choose a reason for hiding this comment

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

Should be indented

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