Skip to content

Conversation

@wankoak
Copy link

@wankoak wankoak commented Oct 23, 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

Added Jest test files for angle type, proper fraction, and card value functions

No questions

@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).

@wankoak wankoak added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 23, 2025
Comment on lines 11 to 21
// Handle invalid denominators (e.g., divide by zero)
if (denominator === 0) {
return false;
}

// Compare absolute values for proper fraction check
if (Math.abs(numerator) < Math.abs(denominator)) {
return true;
} else {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This solution is correct. If you are up to some challenge, try simplify the code to use at most one if-statement.

Comment on lines 25 to 27
if (!isNaN(rank) && Number(rank) >= 2 && Number(rank) <= 9) {
return Number(rank);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In JavaScript, strings that represent valid numeric literals in the language can be safely
converted to equivalent numbers or parsed into a valid integers.
Do you want to recognize these string values as valid ranks?

To find out what these strings are, you can ask AI

What kinds of string values would make Number(rank) evaluate to 2 in JS?

@cjyuan
Copy link
Contributor

cjyuan commented Oct 27, 2025

The PR description is missing a "Changelist' section with a brief description of the changes made in PR.

@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 Oct 27, 2025
@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).

1 similar comment
@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).

@wankoak
Copy link
Author

wankoak commented Oct 29, 2025

@cjyuan I fixed some errors that you mentioned

@wankoak wankoak added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 29, 2025
@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).

@wankoak wankoak removed the Reviewed Volunteer to add when completing a review with trainee action still to take. label Oct 29, 2025
@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).

1 similar comment
@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).

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.

  • The bot's message "Your PR description contained template fields" refers to the placeholder text "Briefly explain your PR" in the PR description. The bot expects you to delete it after you typed your brief description.

Comment on lines +16 to +18
} else if ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11){
return parseInt(rank)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you find out from AI what strings can make the expression ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11) evaluate to true?

Should all of these strings be recognized as valid rank?

Also, what's the purpose of (... && parseInt(rank))?

} else if ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11){
return parseInt(rank)
}
else if( rank === "J" || rank === "Q" || rank === "K") {
Copy link
Contributor

Choose a reason for hiding this comment

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

What you had before is equally good.

if (["K", "Q", "J", "10"].includes(rank)) {

// write one test at a time, and make it pass, build your solution up methodically
// just make one change at a time -- don't rush -- programmers are deep and careful thinkers
function getCardValue(card) {
let rank = card.slice(0, card.length - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why replaced const rank = card.slice(0, -1);?

@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 Oct 29, 2025
@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).

1 similar comment
@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).

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

Labels

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