-
-
Notifications
You must be signed in to change notification settings - Fork 275
Glasgow | 25-ITP-Sep | Hanna Mykytiuk | Sprint 3 | Stretch #816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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). |
| function getAngleType(angle) { | ||
| if (angle === 90) { | ||
| return "Right angle"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HannaOdud Great job! You’ve implemented all five cases correctly. Your logic is solid and the code reads clearly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
| else if (angle > 180 && angle < 360) { | ||
| return "Reflex angle" | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HannaOdud Any thoughts on how we might handle invalid angles (like negatives or values over 360)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have changed it.
| return "Right angle"; | ||
| } | ||
| else if (angle < 90) { | ||
| return "Acute angle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, it’d be a good idea to add semicolons after each return statement (like return "Acute angle";) throughout the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yessss. :-) Thanks!
| if (numerator < denominator) { | ||
| return true; | ||
| } | ||
| return (Math.abs(numerator) < Math.abs(denominator)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!!!!
Well done @HannaOdud
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have changed it.
| if (rank === "A") { | ||
| return 11; | ||
| } | ||
| else if ((parseInt(rank) >= 2 && parseInt(rank)) && parseInt(rank) < 11){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have changed it.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| return 10; | ||
| } | ||
| else{ | ||
| throw new Error("Invalid card rank.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent — throwing an error here makes sense.
Just small formatting notes:
-
Remove the extra space after throw.
-
Add a semicolon at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've changed it.
| // Then it should throw an error indicating "Invalid card rank." | ||
|
|
||
|
|
||
| try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.