London | 26-ITP-May | Vito Moratti | Sprint 3 | Implement and rewrite#1525
London | 26-ITP-May | Vito Moratti | Sprint 3 | Implement and rewrite#1525vmoratti wants to merge 11 commits into
Conversation
vmoratti
added
the
Needs Review
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on these tasks. Some comments for you to look at below
| return "Reflex angle"; | ||
| } else if (acuteAngle) { | ||
| return "Acute angle"; | ||
| } else if (angle === zeroAngle || angle != acuteAngle || angle != rightAngle || angle != obtuseAngle || angle != straightAngle || angle != reflexAngle) { |
There was a problem hiding this comment.
I'm a bit confused about the if condition for this last branch. Can you explain your thinking for me please?
There was a problem hiding this comment.
Well, I thought that i assign conditions to variables and then compare 'angle' parameter to my variables and if it doesn't match, then 'Invalid angle' is returned. I probably should have put this condition in the beginning as an 'if' statement
There was a problem hiding this comment.
Good work with my other comments, this PR is almost done.
I'm still not sure about this task. Look at what you are comparing, angle against expressions from zeroAngle or acuteAngle calculated earlier in the function. Think about the possible values that are contained in these variables. What possible outcomes could you get from this if condition? When will this branch of the if run?
LonMcGregor
added
Reviewed
Simplified the isProperFraction function to return the comparison directly.
Implemented the getAngleType function to classify angles.
Removed TODO comment for test coverage on invalid cards.
Removed TODO comment and added a test case for zero denominator.
vmoratti
added
the
Needs Review
LonMcGregor
removed
the
Needs Review
2 participants
Self checklist
Changelist
I had to rewrite code and tests and make new PR, here it is