Skip to content

London | 25-ITP-May | Tomislav Dukez | Sprint 3 | Implement and Rewrite Tests - #1437

Closed
tomdu3 wants to merge 12 commits into
CodeYourFuture:mainfrom
tomdu3:coursework/sprint-3/1-implement-tests
Closed

London | 25-ITP-May | Tomislav Dukez | Sprint 3 | Implement and Rewrite Tests#1437
tomdu3 wants to merge 12 commits into
CodeYourFuture:mainfrom
tomdu3:coursework/sprint-3/1-implement-tests

Conversation

@tomdu3

@tomdu3 tomdu3 commented Jul 1, 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

Added tasks from the Sprint 3, Implement and Rewrite Tests:

  1. getAngleType
  2. isProperFraction
  3. getCardValue

@tomdu3 tomdu3 added 🏕 Priority Mandatory This work is expected 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 1, 2026
@tomdu3 tomdu3 changed the title London | 25-ITP-May | Tomislav | Sprint 3 | Implement and Rewrite Tests London | 25-ITP-May | Tomislav Dukez | Sprint 3 | Implement and Rewrite Tests Jul 1, 2026
@tomdu3 tomdu3 added the Module-Structuring-And-Testing-Data The name of the module. label Jul 2, 2026
LonMcGregor
LonMcGregor reviewed Jul 6, 2026
View reviewed changes

@LonMcGregor LonMcGregor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work. One comment about your fractions testing, though

Comment thread Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js
@LonMcGregor LonMcGregor 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 Jul 6, 2026
tomdu3
tomdu3 commented Jul 6, 2026
View reviewed changes

@tomdu3 tomdu3 left a comment

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.

@LonMcGregor, Of course I've missed that, and naturally - there's a bug. So, I fixed the issue with calculation and added tests with negative numbers.
Awesome review. Thanks.

@tomdu3 tomdu3 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 Jul 6, 2026
LonMcGregor
LonMcGregor reviewed Jul 7, 2026
View reviewed changes

@LonMcGregor LonMcGregor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work, almost there!

Comment thread Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js Outdated

function isProperFraction(numerator, denominator) {
// TODO: Implement this function
if (denominator === 0 || Math.abs(numerator / denominator) > 1) {

@LonMcGregor LonMcGregor Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work, your test cases for fractions are more expansive now.

My next question is about this if conditional. You are checking something which is a boolean, then returning true/false based on that condition. Can you spot any way you could simplify this code?

@LonMcGregor LonMcGregor 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 Jul 7, 2026
@tomdu3 tomdu3 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 Jul 8, 2026
LonMcGregor
LonMcGregor reviewed Jul 8, 2026
View reviewed changes

@LonMcGregor LonMcGregor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good work fixing this. returning a boolean from an if condition is a common mistake, but it's also pretty easy to fix, so look out for things like this!

@LonMcGregor LonMcGregor removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 8, 2026
@LonMcGregor LonMcGregor added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Jul 8, 2026
This was referenced Jul 17, 2026
Open
Open
@illicitonion

illicitonion commented Aug 24, 2026

Copy link
Copy Markdown
Member

Closing PR because the May ITP run has finished. Feel free to re-open if you're still working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LonMcGregor LonMcGregor LonMcGregor left review comments

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Structuring-And-Testing-Data The name of the module. 🏕 Priority Mandatory This work is expected 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tomdu3 @illicitonion @LonMcGregor