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

Val - Calculator - Edges #45

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

Conversation

valgidzi
Copy link

@valgidzi valgidzi commented Aug 8, 2018

Calculator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe how you stored user input in your program? I stored user input in variables.
How did you determine what operation to perform? I wrote a case statement for the variable storing the operation user input.
Do you feel like you used consistent indentation throughout your code? Yes.
If you had more time, what would you have added to or changed about the program? My program only allows integers. I would change the program to accommodate floats.

@droberts-sea
Copy link

Calculator

What We're Looking For

Feature Feedback
Takes in two numbers and an operator and performs the mathematical operation. yes
Readable code with consistent indentation. yes

Good work overall! You make good use of whitespace to break up your code into sections. I've left a few inline comments below for you to review, but in general I am quite happy with this submission.

# method to verify user input - integer
def integer?
Integer(gets) rescue false
end

Choose a reason for hiding this comment

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

You've named this method integer? with a question mark, which implies that it should return a Boolean (true or false). However it looks like it returns the user's input as an integer. A better name for this method might be something like parse_input.

# check for valid user input
until first
number_reprompt
first = integer?

Choose a reason for hiding this comment

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

You've written almost exactly the same code here twice, to get the first number and the second number. Could you DRY that up by putting this logic in a method?

case op

when "add", "+"
puts "#{first} + #{second} = #{first + second}"

Choose a reason for hiding this comment

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

This code isn't repeated, but I think it would still increase readability to wrap this case/when section in a method. That would clearly delineate where it starts and ends, and make it explicit what data it needs to work. The method signature might be something like perform_calculation(op, first, second).

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