-
-
Notifications
You must be signed in to change notification settings - Fork 195
West Midland | ITP-May-2025 | Saleh Yousef | Module-Structuring-and-Testing-Data | sprint 3 #601
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
new file: prep/example.js
… checks in isProperFraction tests
…date tests for accuracy; complete getAngleType function and enhance test cases for clarity and coverage.
…test cases for negative fractions and equal numerator/denominator scenarios.
…s in a string; add tests for various scenarios including no occurrences. Implement getOrdinalNumber function to return correct ordinal representations; enhance tests for multiple cases including special teen cases.
if (card === "A") return 11; | ||
if (["K", "Q", "J", "10"].includes(card)) return 10; | ||
const num = Number(card); | ||
if (num >= 2 && num <= 9 && card === num.toString()) return num; | ||
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.
Can this function pass all the tests in Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js
?
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.
Previously it wasn’t - now I’ve simplified the getCardValue function to extract the rank using slice(0, -1)
, and cleaned up the logic to handle each valid case cleary.
Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
Outdated
Show resolved
Hide resolved
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.
All the other changes look good.
const num = Number(rank); | ||
if (num >= 2 && num <= 9) return num; |
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.
In JavaScript, strings that represent valid numeric literals in the language can be safely converted to equivalent numbers. For examples, "0x02", "2.1", or "00_02".
Does your function return the value you expected from each of the following function calls?
getCardValue("0x02♠");
getCardValue("2.1♠");
getCardValue("00_02♠");
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.
fixed, the earlier function had loose logic.
test("should return 5 for Five of Hearts", () => { | ||
expect(getCardValue("5♥")).toEqual(5); | ||
expect(getCardValue("3♣")).toEqual(3); | ||
expect(getCardValue("4♦")).toEqual(4); | ||
|
||
|
||
}); |
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.
The test description does not quite match the values being tested.
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.
I’ve fixed the description so it matches the actual test values now. Thank you
Changes look good. Well done! |
Learners, PR Template
Self checklist
Changelist
Completed sprint 3 coursework
Questions
Ask any questions you have for your reviewer.