London | 26-ITP-Jan | Boualem Larbi Djebbour | sprint 3 | implement and rewrite tests coursework#1261
Conversation
Implemented the getAngleType function to classify angles and added tests for various angle types, including acute, obtuse, right, straight, reflex, and invalid angles.
…nk is "10" so the string is 3 characters
…e the rank is "10" so the string is 3 characters" This reverts commit 82958b5.
djebsoft
added
the
Needs Review
| if (card[0] === "A") return 11; | ||
| if (["J", "Q", "K"].includes(card[0])) return 10; | ||
| if (["2", "3", "4", "5", "6", "7", "8", "9", "10"].includes(card[0])) | ||
| return card[0]; // the parseint() or Number() can be used to convert the string to a number | ||
| throw new Error("invalid rank"); |
There was a problem hiding this comment.
Does your function return the value you expected from each of the following function calls?
getCardValue("+2♠");
getCardValue("2.0♠");
getCardValue("11♠");
getCardValue("2 ♠");
getCardValue("XYZ2♠");
There was a problem hiding this comment.
i tested all these sases and yes it does
the return is either invalid suit or invalid rank
but I made some changes in case as follows:
I declared the suit and rank parts as different variables to get the valid results only when the input maches exactly the condition
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/1-get-angle-type.test.js
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js
Show resolved
Hide resolved
cjyuan
added
Reviewed
| test(`should return false for improper fraction`, () => { | ||
| expect(isProperFraction(2, -1)).toEqual(false); | ||
| }); | ||
|
|
||
| // case: both values of the numerator and denominator are negative and the absolute value of the numerator is less than the absolute value of the denominator (proper fraction) | ||
| test(`should return true for proper fraction`, () => { | ||
| expect(isProperFraction(-1, -2)).toEqual(true); | ||
| }); | ||
|
|
||
| // case: both values of the numerator and denominator are negative and the absolute value of the numerator is greater than the absolute value of the denominator (improper fraction) | ||
| test(`should return false for improper fraction`, () => { | ||
| expect(isProperFraction(-2, -1)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
When multiple tests share the same description, a failure can make it harder and slower to identify which specific test failed.
You could consider combine the tests with the same description into the same group.
There was a problem hiding this comment.
the same description, do you mean the same return value or the same status of the denominator and numerator ?
I tryed grouping them based on the return value
There was a problem hiding this comment.
I mean the string value passed to test() as the first argument. When a test fails, that's the string Jest output.
| test("should return the value of number cards (2-10)", () => { | ||
| expect(getCardValue("2♠")).toEqual(2); | ||
| expect(getCardValue("3♥")).toEqual(3); | ||
| expect(getCardValue("4♦")).toEqual(4); | ||
| expect(getCardValue("5♣")).toEqual(5); | ||
| expect(getCardValue("6♥")).toEqual(6); | ||
| expect(getCardValue("7♠")).toEqual(7); | ||
| expect(getCardValue("8♦")).toEqual(8); | ||
| expect(getCardValue("9♠")).toEqual(9); | ||
| expect(getCardValue("10♣")).toEqual(10); | ||
| }); |
There was a problem hiding this comment.
I don't think your function can pass this test yet.
There was a problem hiding this comment.
I changed the returned outcome to a number in the function
return Number(cardRank)
…e card, which can have a value of either 1 or 11 depending on the context of the hand. This allows the function to correctly calculate the value of the hand in blackjack, where the ace can be counted as either 1 or 11 to maximize the player's chances of winning.
djebsoft
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done.
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done.
|
|
||
| // case: both values are positive and the value of the numerater is less than the value of the denominater (proper fraction) | ||
| // Special case: numerator is zero (proper fraction) | ||
| test(`should return true for proper fraction`, () => { |
There was a problem hiding this comment.
The grouping is good.
Could also consider including the condition abs(numerator) < abs(denominator) in the description so that one does not need to lookup the definition of proper fraction.
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
implement and tests functions
Questions