Glasgow | 25-ITP-Sep | Hanna Mykytiuk | Sprint 3 | Stretch#816
Glasgow | 25-ITP-Sep | Hanna Mykytiuk | Sprint 3 | Stretch#816HannaOdud wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
|
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). |
|
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
|
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). |
|
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
|
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). |
| @@ -11,6 +11,19 @@ function getAngleType(angle) { | |||
| if (angle === 90) { | |||
| return "Right angle"; | |||
| } | |||
There was a problem hiding this comment.
@HannaOdud Great job! You’ve implemented all five cases correctly. Your logic is solid and the code reads clearly
| else if (angle > 180 && angle < 360) { | ||
| return "Reflex angle" | ||
| } | ||
|
|
There was a problem hiding this comment.
@HannaOdud Any thoughts on how we might handle invalid angles (like negatives or values over 360)?
| return "Right angle"; | ||
| } | ||
| else if (angle < 90) { | ||
| return "Acute angle" |
There was a problem hiding this comment.
For consistency, it’d be a good idea to add semicolons after each return statement (like return "Acute angle";) throughout the function.
| if (numerator < denominator) { | ||
| return true; | ||
| } | ||
| return (Math.abs(numerator) < Math.abs(denominator)) |
| // 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) |
There was a problem hiding this comment.
@HannaOdud Good use of slice() to get the part before the suit symbol!
A small improvement: you can write it as:
const rank = card.slice(0, -1);
reference:
https://www.w3schools.com/jsref/jsref_slice_string.asp
| if (rank === "A") { | ||
| return 11; | ||
| } | ||
| else if ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11){ |
There was a problem hiding this comment.
You’ve got the right idea here — checking for number cards.
A couple of tips to make it simpler and easier to read:
-
You don’t need to call parseInt several times — you can store it in a variable once.
-
You can also check if rank is a number using !isNaN(rank).
| else if ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11){ | ||
| return parseInt(rank) | ||
| } | ||
| else if( rank === "J" || rank === "Q" || rank === "K") { |
There was a problem hiding this comment.
Nice work catching all the face cards!
Here’s a small tip: you can make this a bit cleaner by using an array:
else if (["J", "Q", "K"].includes(rank))
| return 10; | ||
| } | ||
| else{ | ||
| throw new Error("Invalid card rank.") |
There was a problem hiding this comment.
Excellent — throwing an error here makes sense.
Just small formatting notes:
-
Remove the extra space after throw.
-
Add a semicolon at the end.
| // Then it should throw an error indicating "Invalid card rank." | ||
|
|
||
|
|
||
| try { |
There was a problem hiding this comment.
You’re on the right track with try...catch, but assertEquals won’t run because the function throws an error first.
To actually test that an error happens, you can rewrite it like this:
try {
getCardValue("W♥");
console.log("❌ Test failed: no error thrown");
} catch (error) {
assertEquals(error.message, "Invalid card rank.");
}
This way, you’re checking that the error message matches exactly what your function throws.
|
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
|
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). |
|
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
|
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). |
|
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
|
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). |
Learners, PR Template
Self checklist
Changelist
Stretch tasks.
Questions
Ask any questions you have for your reviewer.