Repository navigation
London |26-ITP-Sep| Maharit Belai| Sprint 2 | Implement and rewrite tests #1716
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?
Changes from all commits
dc81a0f
daed937
fc67493
7218c4e
764fa71
fb8933e
eee5696
873bd2a
971803f
47a15f8
8f94698
dfe51bb
de57932
0c40097
d688c88
bc3a95c
20c2bb0
1e64e02
d645a0c
cfb7d08
f9c3407
68f1eca
0cd8cf7
a618e0e
5cff5b3
589b0d3
4784665
c6355a8
adaacaa
6c09cd2
edbd513
2ea44a4
cfd0d48
ae41393
538f9e4
8fd5918
709e8d3
21f6866
88463d8
9241fc4
7f50065
227e303
f0bb649
26a5e79
c330fd0
c2a7740
9da1f9c
2923430
79e3535
a2f7fb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,5 +24,55 @@ | |
| // execute the code to ensure all tests pass. | ||
|
|
||
| export function getCardValue(card) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overall code is good and works fine, however few things can be improved to make it better.
|
||
| // TODO: Implement this function | ||
| if (card == null) { | ||
| throw new Error(`Expected a valid Rank and Suit instead got null`); | ||
| } | ||
| const suit = card.slice(-1); | ||
| const cardInput = card.length; | ||
| const validSuit = ["♠", "♥", "♣", "♦"]; | ||
| const validRank = [ | ||
| "A", | ||
| "2", | ||
| "3", | ||
| "4", | ||
| "5", | ||
| "6", | ||
| "7", | ||
| "8", | ||
| "9", | ||
| "10", | ||
| "J", | ||
| "Q", | ||
| "K", | ||
| ]; | ||
| if (card == "invalid") { | ||
| throw new Error(`Expected a number followed by a suit, but got "${card}"`); | ||
| } | ||
|
|
||
| if (!validRank.includes(card.slice(0, -1)) && !validSuit.includes(suit)) { | ||
| throw new Error(`Expected a valid Rank and Suit instead got "${card}"`); | ||
| } | ||
| if (!validRank.includes(card.slice(0, -1))) { | ||
| throw new Error(`Expected a valid Rank instead got "${card}"`); | ||
| } | ||
| if (!validSuit.includes(suit)) { | ||
| throw new Error(`Expected a valid Suit instead got "${card}"`); | ||
| } | ||
|
|
||
| if (validSuit) { | ||
| if (cardInput == 2) { | ||
| if (card[0] == "A") { | ||
| return 11; | ||
| } | ||
| if (card[0] == "J" || card[0] == "Q" || card[0] == "K") { | ||
| return 10; | ||
| } | ||
| if (card[0] >= 2 && card[0] < 10) { | ||
| return Number(card[0]); | ||
| } | ||
| } | ||
| if (cardInput == 3 && card.slice(0, 2) === "10") { | ||
| return 10; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,27 @@ import { isProperFraction } from "../implement/2-is-proper-fraction.js"; | |
| test(`should return false when denominator is zero`, () => { | ||
| expect(isProperFraction(1, 0)).toEqual(false); | ||
| }); | ||
| test(`should return true when (denominator > numerator)`, () => { | ||
| expect(isProperFraction(1, 2)).toEqual(true); | ||
| }); | ||
| test(`should return false when (denominator< numerator)`, () => { | ||
| expect(isProperFraction(2, 1)).toEqual(false); | ||
| }); | ||
| test(`should return true when (denominator < numerator)`, () => { | ||
| expect(isProperFraction(1, -2)).toEqual(true); | ||
| }); | ||
| test(`should return false when(numerator = 0)`, () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please review this test case again |
||
| expect(isProperFraction(0, 1)).toEqual(false); | ||
| }); | ||
| test(`should return false when(numerator = 0 and numerator = 0)`, () => { | ||
| expect(isProperFraction(0, 0)).toEqual(false); | ||
| }); | ||
| test(`should return true when(numerator < denominator)`, () => { | ||
| expect(isProperFraction(-1, 2)).toEqual(true); | ||
| }); | ||
| test(`should return true when the input is -ve |numerator|<|denominator|`, () => { | ||
| expect(isProperFraction(-1, -2)).toEqual(true); | ||
| }); | ||
| test(`should return false when the input is -ve |numerator|>|denominator|`, () => { | ||
| expect(isProperFraction(-2, -1)).toEqual(false); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,63 @@ | ||
| import { getCardValue } from "../implement/3-get-card-value.js"; | ||
|
|
||
| // TODO: Write tests in Jest syntax to cover all possible outcomes. | ||
|
|
||
| // Case 1: Ace (A) | ||
| test(`Should return 11 when given an ace card`, () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we've added a ton of validation for checking if the input is correct, wouldn't it make more sense to test it to confirm if its working fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think about malformed inputs etc.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the constructive feedback . I really appreciate it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MaharitB couple of test including malformed inputs like having empty spaces at the start or end of string will make the test suits more robust. Thanks
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the clarification . i have made the changes based on your advice. |
||
| expect(getCardValue("A♠")).toEqual(11); | ||
| }); | ||
| test(`should retun the numeric value when given "numbered" cards`, () => { | ||
| expect(getCardValue("2♣")).toEqual(2); | ||
| expect(getCardValue("6♥")).toEqual(6); | ||
| expect(getCardValue("10♦")).toEqual(10); | ||
| }); | ||
| test(`should retun 10 when given(J,Q,K)cards`, () => { | ||
| expect(getCardValue("J♣")).toEqual(10); | ||
| expect(getCardValue("Q♥")).toEqual(10); | ||
| expect(getCardValue("K♦")).toEqual(10); | ||
| }); | ||
| test(`throws for invalid cards`, () => { | ||
| expect(() => { | ||
| getCardValue("invalid"); | ||
| }).toThrow(`Expected a number followed by a suit, but got "invalid"`); | ||
| }); | ||
| test(`throws for invalid rank`, () => { | ||
| expect(() => { | ||
| getCardValue("1♥"); | ||
| }).toThrow(`Expected a valid Rank instead got "1♥"`); | ||
| }); | ||
| test(`throws for invalid suit`, () => { | ||
| expect(() => { | ||
| getCardValue("AS"); | ||
| }).toThrow(`Expected a valid Suit instead got "AS"`); | ||
| }); | ||
| test(`throws for invalid input`, () => { | ||
| expect(() => { | ||
| getCardValue("100"); | ||
| }).toThrow(`Expected a valid Suit instead got "100"`); | ||
| }); | ||
| test(`throws for invalid rank`, () => { | ||
| expect(() => { | ||
| getCardValue("15♥"); | ||
| }).toThrow(`Expected a valid Rank instead got "15♥"`); | ||
| }); | ||
| test(`throws for invalid input`, () => { | ||
| expect(() => { | ||
| getCardValue(" "); | ||
| }).toThrow(`Expected a valid Rank and Suit instead got " "`); | ||
| }); | ||
| test(`throws for invalid input with space ending`, () => { | ||
| expect(() => { | ||
| getCardValue("4♣ "); | ||
| }).toThrow(`Expected a valid Rank and Suit instead got "4♣ "`); | ||
| }); | ||
| test(`throws for invalid input`, () => { | ||
| expect(() => { | ||
| getCardValue("♣4"); | ||
| }).toThrow(`Expected a valid Rank and Suit instead got "♣4"`); | ||
| }); | ||
| test(`throws for invalid input`, () => { | ||
| expect(() => { | ||
| getCardValue(null); | ||
| }).toThrow(`Expected a valid Rank and Suit instead got null`); | ||
| }); | ||
|
|
||
| // Suggestion: Group the remaining test data into these categories: | ||
| // Number Cards (2-10) | ||
|
|
@@ -15,4 +67,3 @@ test(`Should return 11 when given an ace card`, () => { | |
| // To learn how to test whether a function throws an error as expected in Jest, | ||
| // please refer to the Jest documentation: | ||
| // https://jestjs.io/docs/expect#tothrowerror | ||
|
|
||
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.
is null and empty space the same in javascript?
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.
@hey-hammad No,they are not the same. empty space is considered a character and null is nothing.
I will have a look on the code. Thank you for raising it.