Skip to content

Conversation

@i786m
Copy link

@i786m i786m commented Oct 22, 2025

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

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

@github-actions
Copy link

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 i786m changed the title London | 25- | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite London | 25-ITP-SEP | Imran Mohamed| Sprint 3 | Coursework/sprint 3 implement and rewrite Oct 22, 2025
@i786m i786m added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 22, 2025
Copy link

@A-O-Emmanuel A-O-Emmanuel Oct 26, 2025

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

@A-O-Emmanuel A-O-Emmanuel added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 26, 2025
}
if(numerator == 0){
return true;
}
Copy link
Author

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.

@i786m i786m added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 28, 2025
@i786m i786m requested a review from A-O-Emmanuel October 28, 2025 11:02
@A-O-Emmanuel A-O-Emmanuel added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 28, 2025
@i786m
Copy link
Author

i786m commented Nov 1, 2025

fixed the typo in the condition and confirmed all tests are passing before committing. Let me know if there are any other issues

@i786m i786m added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 1, 2025
@A-O-Emmanuel A-O-Emmanuel added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Nov 14, 2025
@i786m
Copy link
Author

i786m commented Nov 18, 2025

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.

@A-O-Emmanuel
Copy link

Look at the comments provided and act on them

@i786m
Copy link
Author

i786m commented Nov 18, 2025

image 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

@A-O-Emmanuel
Copy link

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.

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.

Copy link
Author

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

Choose a reason for hiding this comment

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

The test are okay

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

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

Copy link
Author

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;

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;
}

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;
}

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;
}

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"

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.

Copy link
Author

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

@i786m i786m added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 22, 2025
@i786m i786m requested a review from A-O-Emmanuel November 22, 2025 12:21
@A-O-Emmanuel A-O-Emmanuel added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants