-
-
Notifications
You must be signed in to change notification settings - Fork 244
London | 25-ITP-SEP | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite #791
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
base: main
Are you sure you want to change the base?
London | 25-ITP-SEP | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite #791
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). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your implementation of the getAngleType function is logically correct, and follow TDD principles good work
| } | ||
| if(numerator == 0){ | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
fixed the typo in the condition and confirmed all tests are passing before committing. Let me know if there are any other issues |
|
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should review the acceptance criteria again for this and follow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amended to throw error as opposed to returning for invalid cards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test are okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test structure is okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches acceptance criteria 1 which is "proper fraction check"
| } | ||
| if (numerator >= denominator) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches acceptance criteria 2 which is "improper fraction check"
| } | ||
| if(numerator == 0 && denominator !== 0){ | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches stretch scenario 1 which "Zero numerator check"
| } | ||
| if (denominator === 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches stretch scenario 2 which is "zero denominator check"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amended to address the remaining 2 cases in acceptance criteria

Self checklist
Changelist
I have coded the respective functions, assertions and tests for this portion of coursework