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

BarbaraWidjono, Edges, calculator.rb #41

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

Conversation

BarbaraWidjono
Copy link

Calculator

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Describe how you stored user input in your program? User input stored in variables.
How did you determine what operation to perform? User inputs operation (add, +, etc), that operation is saved into a variable.
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? Checking the user inputed valid numbers for use in calculations.

@tildeee
Copy link
Collaborator

tildeee commented Aug 12, 2018

Calculator

What We're Looking For

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

Good work on this project!

The project's code is readable and straightforward and a good solution, which is my favorite kind of solution :) While you didn't get a chance to add in checking that the user input for numbers for validity, I think you still hit all the learning goals-- practicing making methods, working with complex conditionals, etc.

If I were to make a major suggestion, it would be to try using a case statement ( Read more here )

I'm adding a few comments, mostly on small formatting suggestions

In general, good work on this code :)

return (x * y).to_f
end

def division (x , y)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though the code still runs, most style guides, style conventions, and text editors like Atom will complain/give a warning if there's a space between the method name and the parentheses. aka:

def division (x , y)

should be this:

def division(x , y)


# Determining which method to invoke depending on the operand chosen by the user
if operand == "add" || operand == "+"
puts "Answer: #{first_value} + #{second_value} = #{addittion(first_value, second_value)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like how concise these lines are: You are interpolating things and calling methods in them -- it shows that you understand that many method calls can happen on one line, which is a crucial piece of understanding!

end

def subtraction(x, y)
return (x - y).to_f
Copy link
Collaborator

Choose a reason for hiding this comment

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

you end up calling .to_f on this result (and in some other calculation methods).

However, let's establish some things:

  • in lines 21 and 26, you get the values for first_value and second_value by getting user input and then converting them to floats immediately with .to_f.

And let's remember:

  • in Ruby, if you do addition, subtraction, multiplication, or division with any floats, the result will be a float

Therefore: do you need to convert to a float here?

Sometimes, it's good to have a .to_f in here just in case there's a chance you still need to convert it, but within this particular program, you probably wouldn't need to call .to_f here. Not a big deal! Just wanted to call out this redundancy.

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