Conversation
beccaelenzil
left a comment
beccaelenzil
left a comment
There was a problem hiding this comment.
Your code meets the required specifications and you have used a clever method to check for valid user input. The user experience when there is valid user input is quite refined. A few things to consider:
- Use meaningful variable names to make your code more readable/understandable
- Avoid repeating code (DRY) by using methods and loops
- Use when/case and if/elsif/else to simplify complicated data structures
- Use while or until to continually check for valid user input rather than exiting the program when you receive invalid input.
See specific comments in the code review.
| @@ -0,0 +1,55 @@ | |||
| # 1st method - evaluate_oper(operator): evaluate user input to see if it is one of the four avaliable operators (either symbol or word) for this calculator program. If yes, method will return the input; if no, method will display error message. Assumption: only lowercase input | |||
| def evaluate_oper(operator) | |||
| if ["add", "+", "subtract", "-", "multiply", "*", "divide", "/"].include?(operator) | |||
There was a problem hiding this comment.
Consider using a while loop to continually ask for user input until it is valid.
| end | ||
| end | ||
| # 2nd method - evaluate_num(number): evaluate user input to see if it is a number. Prior - all input are strings; Post - all input are converted into float [number will remain float; letter will become 0; expect for integer 0]. Assumption: don't need to handle 0 for this assignment *** TA Kaida helped me on this method | ||
| def evaluate_num(number) |
There was a problem hiding this comment.
Similar to evaluate_op, consider using a while loop to continually ask for user input until it is valid.
| end | ||
| end | ||
| # 3rd method - operation(a,b,c): take variable a to compare against the four available operators and return corresponding calculation of numbers b and c. Assumption: input for a is one of the four available operators | ||
| def operation(a,b,c) |
There was a problem hiding this comment.
Consider using a case/when block to simplify your code.
| puts "Weclome to my calculator!\nPlease choose one operator from below (either name or symbol;lowercase only please).\n1. add(+)\n2. substract(-)\n3. multiply(*)\n4. divide(/) \n" | ||
| puts "What is your chosen operator?" | ||
| input1 = gets.chomp | ||
| result1 = evaluate_oper(input1) |
There was a problem hiding this comment.
It is advisable to use meaningful variable names. Instead of result1, result2, result3, how about operation, num1, and num2?
| input1 = gets.chomp | ||
| result1 = evaluate_oper(input1) | ||
| if result1 == "This is an invalid input" | ||
| puts "Invalid input. Bye Bye" |
There was a problem hiding this comment.
You repeat this exact line (puts "Invalid input. Bye bye") many times. Consider how you could use methods and loops to DRY up your code.
| result2 = evaluate_num(input2) | ||
| if result2 == "This is an invalid input" | ||
| puts "Invalid input. Bye Bye" | ||
| else |
There was a problem hiding this comment.
This conditional control structure has many nested layer. Could you simplify your code with if/elif/else or with the use of methods?
CalculatorWhat We're Looking For
|
2 participants
Calculator
Congratulations! You're submitting your assignment.
Comprehension Questions