-
-
Notifications
You must be signed in to change notification settings - Fork 391
London | May-2026-itp | Vitalii Kmit | Sprint 3 | Implement and rewrite tests #1463
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
8a35211
bfa68d6
043de8e
023e69f
92027a9
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 |
|---|---|---|
|
|
@@ -22,7 +22,34 @@ | |
| // execute the code to ensure all tests pass. | ||
|
|
||
| function getCardValue(card) { | ||
| // TODO: Implement this function | ||
| const lastCharIndex = card.length - 1; | ||
| const rank = card.slice(0, lastCharIndex); | ||
| const suit = card.slice(lastCharIndex); | ||
|
|
||
| if (rank === "10") { | ||
| return 10; | ||
| } | ||
|
|
||
| if (card.length > 2) { | ||
| throw new Error("Invalid input length"); | ||
| } | ||
|
|
||
| if (!(suit === "♠" || suit === "♥" || suit === "♦" || suit === "♣")) { | ||
| throw new Error( | ||
|
Member
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 don't think this works like you think it does. Can you update your tests to show that the error message is what you expect it to be? |
||
| "Invalid second character detected, only suits(♠♥♦♣) are allowed, but got this character:", | ||
| suit | ||
| ); | ||
| } | ||
|
|
||
| if (rank == "A") { | ||
| return 11; | ||
| } else if (rank == "J" || rank == "Q" || rank == "K") { | ||
|
Comment on lines
+44
to
+46
Member
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. Why the |
||
| return 10; | ||
| } else if (!isNaN(rank)) { | ||
| return Number(rank); | ||
| } else { | ||
| throw new Error("Invalid card"); | ||
|
Member
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. This also has the same "what was wrong?" problem |
||
| } | ||
| } | ||
|
|
||
| // The line below allows us to load the getCardValue function into tests in other files. | ||
|
|
||
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 works, but I'm curious - why did you decide to handle 10 specially here?
What would happen if someone passed the string
"10Q"to this function? What would you want to happen? What other edge cases may you want to test for in your tests?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.
Well because "10♠" is three characters so I decided to write a special case for it, otherwise all the others combinations should be something like this "K♠" so only two characters. It's probably not the best way to handle this but it works 🤷
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.
It works for half of this ("Don't have to worry about 3-character strings after this line"), but it doesn't work for e.g. the
"10Q"example. Can you talk through what you expect to happen if someone passes"10Q"and what your code actually does?