Skip to content

London | 26-ITP-May | Chinwe Chukwuma | Sprint 3 | implement-and-rewrite - #1573

Open
ChinweP wants to merge 10 commits into
CodeYourFuture:mainfrom
ChinweP:coursework/sprint-3-implement-and-rewrite
Open

London | 26-ITP-May | Chinwe Chukwuma | Sprint 3 | implement-and-rewrite#1573
ChinweP wants to merge 10 commits into
CodeYourFuture:mainfrom
ChinweP:coursework/sprint-3-implement-and-rewrite

Conversation

@ChinweP

@ChinweP ChinweP commented Jul 26, 2026

Copy link
Copy Markdown

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 coursework/implement-and-rewrite following the ReadMe instruction.

@ChinweP ChinweP added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Jul 26, 2026
@cjyuan cjyuan added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jul 27, 2026
Comment on lines +15 to +18
if (denominator <= 0) {
return false;
}
return numerator < denominator;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What authoritative source do you base your definition of a proper fraction on?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you @cjyuan for your feedback. I have added authoritative source and made necessary adjustments.

});

// Case 6: Invalid angles
test(`should return "Invalid angle" for angles outside valid range`, () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could consider indicate explicitly what the valid range is.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @cjyuan, I've made the clarification. Thank you for your feedback.

@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. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 27, 2026
@ChinweP ChinweP added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 29, 2026
@ChinweP
ChinweP requested a review from cjyuan July 29, 2026 00:46
});

// Case 6: Invalid angles
test(`should return "Invalid angle" for angles outside valid range (1-359)`, () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could the value of angle be a decimal number? For example, is 0.5 a valid angle?

Comment on lines +26 to +28
- The denominator is positive.

Source: https://www.mathsisfun.com/proper-fractions.html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: The source does not actually say anything about negative numerator or denominator.

No change required because the spec also does not say anything about what to do with negative numbers.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Structuring-And-Testing-Data The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants