Skip to content

Conversation

@Steph0088
Copy link

@Steph0088 Steph0088 commented Aug 7, 2019

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? In terms of readability it is pretty straight forward and I commented throughout. My code does needs some modifications that I didn't have time to incorporate or figure out, like having my array print out the operations instead of putting puts and printing things out. Also, I didn't have time to add in methods, which I would have preferred having in my case statement.
How did your code keep track of user input? It stored user input in variables num_one and num_two.
How did your code determine what operation to perform? It used a case statement to determine the operation to perform.
What opportunities exist to create small methods for this project? In my case statement I could call methods that I create at the beginning of my code. The methods could do all the math and would look like: def add(num, num){ (num + num) return answer end}
In the next project, what would you change about your process? What would you keep doing? I would write in Pseudocode by commenting it and then fill it in.

@Steph0088 Steph0088 changed the title branches-steph Branches-Steph 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, one small part was off
Practices using variables appropriately x
Practices using conditionals appropriately x
If any, practices iteration appropriately x
If any, practices using custom methods appropriately this submission did not have any methods
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

Well done on this project Steph! You did a great job practicing some interesting syntax we've learned, as well as working with the user input.

If you had more time on this, I would encourage you to find ways to refactor the while loop you have, and find opportunities for building methods. I've left also a few comments on a few other minor suggestions. I may also suggest that by default, use puts rather than print because puts will always give you another line break, which makes your Terminal output more easy to read.

Overall, good work!

puts "3. mulitply(*)"
puts "4. divide(/)"

# Ask user to describe what operation performed. YOu will have to build in a feature that
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fantastic comments to describe what is going on right now :)

puts "Please enter a correct operation"
user_choice = gets.chomp
end
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 like your strategy of using choice here in this while loop! It looks like choice will always be a boolean, and it will be false if the operator is invalid, and true if it is valid and successful. Here, you use a while loop AND an if/else statement. Could we use the while loops condition and the if condition, and consider combining them? Consider something like:

while !operations.include?(user_choice)
  puts "Please enter a correct operation"
  user_choice = gets.chomp
end

This is saying "while the operations array does NOT include the value of user_choice, then that means that the user_choice is not valid, and so we ask for more user input. If the operations array DID include user_choice, then it was valid, and the computer will move past this code"

print "What is your first number: "
begin
num_one = gets.chomp
num_one= Integer(num_one)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is such a small detail, but I really like having spaces around my =, so num_one = Integer(num_one)

rescue ArgumentError
print "Please enter a number:"
retry
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll learn so much more about begin ... rescue clauses later in the program! I think it's okay to use begin ... rescue clauses right now if you'd like, but I could imagine this code being refactored into a while loop if you also refactored some logic

print "Please enter a number:"
retry
end
print num_one
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because you print num_one here, it ends up looking awkward in the terminal, because num_one gets displayed even though I (the user) just typed it! Maybe we can get rid of this line?

print "Please enter a number:"
retry
end
print num_two
Copy link
Collaborator

Choose a reason for hiding this comment

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

Watch your indentation here! It looks like your indentation got off-- rescue and end should not be indented

print "Please enter a number:"
retry
end
print num_two
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above about print num_two here: Because you print num_two here, it looks awkward in the terminal, because num_two gets displayed even though I (the user) just typed it! Maybe we can get rid of this line?

when "divide", "/"
solution = (num_one/num_two).to_f
puts "#{solution}"
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nicely done on the case statement!

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