Skip to content

Sheffield | 26-Jan-ITP | Daniel Aderibigbe | Sprint 3 | Implement and rewrite test done#1200

Open
Dan2Clouted wants to merge 3 commits intoCodeYourFuture:mainfrom
Dan2Clouted:coursework/sprint-3-implement-and-rewrite
Open

Sheffield | 26-Jan-ITP | Daniel Aderibigbe | Sprint 3 | Implement and rewrite test done#1200
Dan2Clouted wants to merge 3 commits intoCodeYourFuture:mainfrom
Dan2Clouted:coursework/sprint-3-implement-and-rewrite

Conversation

@Dan2Clouted
Copy link

Learners, PR Template

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

Completed all the exercisces required.

@Dan2Clouted Dan2Clouted added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 6, 2026
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Function implementations look good.

This exercise has a second part "2 Rewrite tests with Jest" ==> three more files to be updated.

@cjyuan cjyuan 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 Mar 11, 2026
@Dan2Clouted
Copy link
Author

The have been updated now. Thanks in Advance

@Dan2Clouted Dan2Clouted added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 11, 2026
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Changes look good. Just a few suggestions.

I will mark this as Complete first.

Comment on lines +43 to +46
test(`should return "Invalid angle" when angle < 0 or angle >= 360`, () => {
expect(getAngleType(-1)).toEqual("Invalid angle");
expect(getAngleType(360)).toEqual("Invalid angle");
});
Copy link
Contributor

Choose a reason for hiding this comment

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

It's good that you tested one of the boundary cases. You could consider testing the other one.

Comment on lines +21 to +24
test(`should return true when the absolute value of the numerator is smaller than the absolute value of the denominator`, () => {
expect(isProperFraction(-1, 2)).toEqual(true);
expect(isProperFraction(1, -2)).toEqual(true);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This category probably cover the positive case, and you can use notations like | ... | or abs(...) in the description to make it more concise.

@cjyuan cjyuan 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. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 11, 2026
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