London | 25-ITP-SEP | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite#791
London | 25-ITP-SEP | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite#791i786m wants to merge 11 commits intoCodeYourFuture:mainfrom
Conversation
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
i786m
changed the title
i786m
added
the
Needs Review
There was a problem hiding this comment.
I think your implementation of the getAngleType function is logically correct, and follow TDD principles good work
A-O-Emmanuel
added
Review in progress
| } | ||
| if(numerator == 0){ | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This is a stretch scenario I created, I have updated the code to ensure the condition also checks that the denominator is non-zero.
Please let me know if this is sufficient.
i786m
added
Needs Review
A-O-Emmanuel
added
Reviewed
|
fixed the typo in the condition and confirmed all tests are passing before committing. Let me know if there are any other issues |
i786m
added
Needs Review
A-O-Emmanuel
added
Review in progress
|
Is there anything in particular that is preventing me from achieving completion on this PR, I am unsure if there are additional changes that I need to make to get complete label attached in order to submit my work for this module. Please advise. |
|
Look at the comments provided and act on them |
I seem to be unable to access your initial comment about the is-proper-fraction. If I recall correctly, the comment pertained to the zero numerator scenario. Could you kindly recommend what needs to be addressed
|
|
I have already comment, look at the comments and do it, what your are posting is not the only comment i made, just look all the comments. |
There was a problem hiding this comment.
I think you should review the acceptance criteria again for this and follow it.
There was a problem hiding this comment.
amended to throw error as opposed to returning for invalid cards
There was a problem hiding this comment.
The test structure here looks okay, just confirm that the get-card-value.js follows the acceptance criteria
There was a problem hiding this comment.
amended to throw error as opposed to returning error for invalid cards
| function isProperFraction(numerator, denominator) { | ||
| if (numerator < denominator) { | ||
| if (Math.abs(numerator) < Math.abs(denominator)) { | ||
| return true; |
There was a problem hiding this comment.
This matches acceptance criteria 1 which is "proper fraction check"
| } | ||
| if (numerator >= denominator) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This matches acceptance criteria 2 which is "improper fraction check"
| } | ||
| if(numerator == 0 && denominator !== 0){ | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This matches stretch scenario 1 which "Zero numerator check"
| } | ||
| if (denominator === 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This matches stretch scenario 2 which is "zero denominator check"
There was a problem hiding this comment.
You need to work on the other two acceptance criterias not accounted for. The instruction says: "complete the rest of the tests and cases", you have completed two cases in the acceptance criteria and two cases in the strech, you need to complete the remaining two cases in the acceptance criteria.
There was a problem hiding this comment.
amended to address the remaining 2 cases in acceptance criteria
i786m
added
Needs Review
A-O-Emmanuel
added
Complete
3 participants
Self checklist
Changelist
I have coded the respective functions, assertions and tests for this portion of coursework