-
-
Notifications
You must be signed in to change notification settings - Fork 397
London | 26 - ITP-MAY | Ebrahim Moqbel | Sprint 3 | Implement and ewrite tests #1586
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
Changes from 9 commits
9eed6df
49b88ad
3c92b48
bfb2769
34ae0f2
5237b54
e75c8ba
12433ce
c9d4307
7ba78d5
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 |
|---|---|---|
|
|
@@ -23,8 +23,28 @@ | |
|
|
||
| function getCardValue(card) { | ||
| // TODO: Implement this function | ||
| const rank = card.slice(0,-1); | ||
| const cardFace = card[card.length - 1]; | ||
|
|
||
| if (!["♠", "♥", "♦", "♣"].includes(cardFace)) { | ||
| throw new Error(`Invalid card face: ${cardFace}`); | ||
| } | ||
| if (rank === "A") { | ||
| return 11; | ||
| } | ||
| if (+rank >= 2 && +rank <= 9) { | ||
| return +rank; | ||
| } | ||
| if (["K", "10", "Q", "J"].includes(rank)) { | ||
| return 10; | ||
| } | ||
| if (!["♠", "♥", "♦", "♣"].includes(cardFace)) { | ||
| throw new Error(`Invalid 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 indendation of this line is not consistent with the rest of the code. How can you ensure to always format your code consistently? What happens if the condition is false? |
||
|
|
||
|
|
||
|
|
||
|
Comment on lines
46
to
+48
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. I recommend installing the Prettier extension to automatically format the code when saving a file. These new lines are not needed |
||
|
|
||
| // The line below allows us to load the getCardValue function into tests in other files. | ||
| // This will be useful in the "rewrite tests with jest" step. | ||
| module.exports = getCardValue; | ||
|
|
@@ -39,9 +59,42 @@ function assertEquals(actualOutput, targetOutput) { | |
|
|
||
| // TODO: Write tests to cover all outcomes, including throwing errors for invalid cards. | ||
| // Examples: | ||
| assertEquals(getCardValue("9♠"), 9); | ||
| // (Ace All suits) | ||
| const aceOfSpades = getCardValue("A♠"); | ||
| assertEquals(aceOfSpades, 11); | ||
|
|
||
| const aceOfHearts = getCardValue("A♥"); | ||
| assertEquals(aceOfHearts, 11); | ||
|
|
||
| const aceOfDiamonds = getCardValue("A♦"); | ||
| assertEquals(aceOfDiamonds, 11); | ||
|
|
||
| const aceOfClubs = getCardValue("A♣"); | ||
| assertEquals(aceOfClubs, 11); | ||
|
|
||
| // (Face cards) | ||
| const jackOfHearts = getCardValue("J♥"); | ||
| assertEquals(jackOfHearts, 10); | ||
|
|
||
| const queenOfClubs = getCardValue("Q♣"); | ||
| assertEquals(queenOfClubs, 10); | ||
|
|
||
| const kingOfSpades = getCardValue("K♠"); | ||
| assertEquals(kingOfSpades, 10); | ||
|
|
||
| // (Number cards) | ||
| const threeOfDiamonds = getCardValue("3♦"); | ||
| assertEquals(threeOfDiamonds, 3); | ||
|
|
||
| const sevenOfClubs = getCardValue("7♣"); | ||
| assertEquals(sevenOfClubs, 7); | ||
|
|
||
|
|
||
|
|
||
| // Handling invalid cards | ||
| // giving an invalid rank (a number or an recognized face card) | ||
| // When the function is called with such a card, | ||
| // Then it should throw an error indicating "Invalid card rank." | ||
| try { | ||
| getCardValue("invalid"); | ||
|
|
||
|
|
@@ -52,3 +105,17 @@ try { | |
| } | ||
|
|
||
| // What other invalid card cases can you think of? | ||
|
|
||
| try { | ||
| getCardValue("1♠"); | ||
| console.error("Error was not thrown for invalid card rank"); | ||
| } catch (error) { | ||
| assertEquals(error.message, "Invalid card rank"); | ||
| } | ||
|
|
||
| try { | ||
| getCardValue("5X"); | ||
| console.error("Error was not thrown for invalid card suit"); | ||
| } catch (error) { | ||
| assertEquals(error.message, "Invalid card suit"); | ||
| } | ||
|
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. Well done on writing test cases for the border cases. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,24 @@ test(`Should return 11 when given an ace card`, () => { | |
|
|
||
| // Suggestion: Group the remaining test data into these categories: | ||
| // Number Cards (2-10) | ||
| test(`Should return the correct value for number cards (2-10)`, () => { | ||
| expect(getCardValue("2♠")).toEqual(2); | ||
| expect(getCardValue("5♠")).toEqual(5); | ||
| expect(getCardValue("10♠")).toEqual(10); | ||
| }) | ||
| // Face Cards (J, Q, K) | ||
| test(`Should return 10 when given a face card (J, Q, K)`, () => { | ||
| expect(getCardValue("J♠")).toEqual(10); | ||
| expect(getCardValue("Q♠")).toEqual(10); | ||
| expect(getCardValue("K♠")).toEqual(10); | ||
| }) | ||
| // Invalid Cards | ||
| test(`Should throw an error when given an invalid 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. What other invalid cases could you test (which other ranks are invalid and what about a missing rank)
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. I have added another case in line 27 for testing a missing rank. other invalid cases could be negative ranks. |
||
| expect(() => getCartValue("♠").toThrow("Invalid card")); | ||
| expect(() => getCardValue("5X")).toThrow("Invalid card "); //invalid suit | ||
| expect(() => getCardValue("1♠")).toThrow("Invalid card "); //invalid rank | ||
| expect(() => getCardValue("3")).toThrow("Invalid card "); //missing suit | ||
| }) | ||
|
Comment on lines
+26
to
+31
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. Vs code shows me an error here. How can you fix it?
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. there was a closing curly bracket and parenthese in line 27. |
||
|
|
||
| // To learn how to test whether a function throws an error as expected in Jest, | ||
| // please refer to the Jest documentation: | ||
|
|
||
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.
What is the purpose of
+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.
converting the string rank to a number.
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.
This happens 3 times. How could you write the code so the conversion is only done once?
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 meant you could store the rank converted into a number into a variable but this works as well.