LON | ITP JAN-25 | Amir Montazeri | Module Structuring And Data | Sprint3 tasks | Week 3 #314
Conversation
Amirmontazerii
added
the
Needs Review
cjyuan
added
the
Review in progress
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Look great!
getCardValue(card) could be improved to deal with more invalid cases.
Feel free to mark this as completed when you have made the necessary change.
| if (angle === 90) return "Right angle"; | ||
| if (angle < 90) return "Acute angle"; | ||
| if (angle > 90 && angle < 180) return "Obtuse angle"; | ||
| if (angle === 180) return "Straight angle"; | ||
| if (angle > 180 && angle < 360) return "Reflex angle"; | ||
| } |
There was a problem hiding this comment.
The spec does not specify what to do when angle >= 360. How would you handle such a case if it were up to you?
There was a problem hiding this comment.
@cjyuan Thanks for your review! I've updated the function to handle additional cases.
| const rank = card.slice(0, -1); | ||
| if (rank === "A") return 11; | ||
| if (["K", "Q", "J", "10"].includes(rank)) return 10; | ||
| if (!isNaN(rank) && rank >= 2 && rank <= 9) return Number(rank); |
There was a problem hiding this comment.
Does your function return the value you expected from each of the following function calls?
getCardValue("010♠");
getCardValue("02♠");
getCardValue("0x02♠");
getCardValue("2.1♠")
There was a problem hiding this comment.
I updated the function so now It shows that the function throwing errors for invalid card inputs .
| @@ -1,5 +1,5 @@ | |||
| function countChar(stringOfCharacters, findCharacter) { | |||
| return 5 | |||
| return stringOfCharacters.split(findCharacter).length - 1; | |||
There was a problem hiding this comment.
Very interesting and elegant approach!
Since a character is of type "string" in JS, how would you modify your approach to deal with cases when findCharacter is "abc" or ""?
There was a problem hiding this comment.
The current approach works for single characters but can also handle multi-character substrings, However, there are some edge cases, so improved the function to handle theme.
cjyuan
added
Reviewed
Amirmontazerii
added
the
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.