Sheffield | May-2025 | WALEED-YAHYA SALIH-TAHA | Sprint-3 #652
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2. identified obtuse angle and it's function 3. identified stright angle and it's function 4. identified reflex anglewith it's function.
2. Identified Obtuse angles when it's greater than 90 degrees and less than 180, and added a line to test the function. 3. added a line to test all the if conditions in the program.
…odule-Structuring-and-Testing-Data into Sprint3-Test-branch
… line code to check the result.
…o it to handle diffrent type of the cards.
…odule-Structuring-and-Testing-Data into Sprint3-Test-branch
2. tested other scenarios like Zero numerator
…mplement. 2. Added some test examples.
Bluejay600
added
Needs Review
cjyuan
reviewed
Jul 22, 2025
| const rank = card.slice(0, -1); // Get the rank (everything except the last character) | ||
| if (rank === "A") return 11; | ||
| if (["J", "Q", "K"].includes(rank)) return 10; | ||
| if (rank >= "2" && rank <= "9") return parseInt(rank, 10); |
There was a problem hiding this comment.
If the function is not returning what you expect, that means your test or/and the function are incorrect and should be updated accordingly.
Comment on lines
4
to
8
| test("should return 5 for 5 of Hearts", () => { | ||
| const fiveofHearts = getCardValue("5♥"); | ||
| expect(fiveofHearts).toEqual(5); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can you also apply the change if you recognise the suggested approach is better?
Comment on lines
10
to
21
| test("should return 10 for Jack of Diamonds", () => { | ||
| const jackOfDiamonds = getCardValue("J♦"); | ||
| expect(jackOfDiamonds).toEqual(10); | ||
| }); | ||
| test("should return 10 for Queen of Clubs", () => { | ||
| const queenOfClubs = getCardValue("Q♣"); | ||
| expect(queenOfClubs).toEqual(10); | ||
| }); | ||
| test("should return 10 for King of Spades", () => { | ||
| const kingOfSpades = getCardValue("K♠"); | ||
| expect(kingOfSpades).toEqual(10); | ||
| }); |
There was a problem hiding this comment.
I don't see any change in this file since I last reviewed it. Did you forget to commit your changes or push them to Github?
Comment on lines
1
to
10
| function countChar(stringOfCharacters, findCharacter) { | ||
| // Initialize a counter | ||
| let count = 0; | ||
|
|
||
| // Scenario: Multiple Occurrences | ||
| // Given the input string str, | ||
| // And a character char that may occur multiple times with overlaps within str (e.g., 'a' in 'aaaaa'), | ||
| // When the function is called with these inputs, | ||
| // Then it should correctly count overlapping occurrences of char (e.g., 'a' appears five times in 'aaaaa'). | ||
| // Loop through the string | ||
| for (let char of stringOfCharacters) { | ||
| if (char === findCharacter) { | ||
| count++; | ||
| } | ||
| } |
There was a problem hiding this comment.
You can import the function from count.js as
const countChar = require("./count");
Sprint-3/3-mandatory-practice/implement/get-ordinal-number.test.js
Outdated
Show resolved
Hide resolved
cjyuan
added
Reviewed
2. Added all required checks according to your criteria. 3. Used the allowed special characters only: !, #, $, %, ., *, &.
2. edited the rank to be (2_9) only to ensures only one-digit numeric strings are parsed.
…works too, but toBe() is stricter).
2. Added a test for "10" card (which sometimes causes issues if not handled early).
3. Added extra invalid card checks ("" and "10" without suit) to catch edge cases.
4. Included "A♠" for completeness under the Ace tests.
Bluejay600
added
Needs Review
cjyuan
reviewed
Jul 25, 2025
Comment on lines
+3
to
+10
| // Case 1: Handle Ace (A) | ||
| test("should return 11 for Ace of Spades", () => { | ||
| const aceofSpades = getCardValue("A♠"); | ||
| expect(aceofSpades).toEqual(11); | ||
| }); | ||
|
|
||
| // Case 2: Handle Number Cards (2-10): | ||
| // Case 3: Handle Face Cards (J, Q, K): | ||
| // Case 4: Handle Ace (A): | ||
| // Case 5: Handle Invalid Cards: | ||
| expect(getCardValue("A♠")).toBe(11); | ||
| }); | ||
|
|
||
| test("should return 11 for Ace of Diamonds", () => { | ||
| expect(getCardValue("A♦")).toBe(11); | ||
| }); |
There was a problem hiding this comment.
Why do you set up two tests for Ace when the function is not expected to check the suit?
| }); | ||
|
|
||
| test("should return 10 for 10 of Clubs", () => { | ||
| expect(getCardValue("10♣")).toBe(10); |
There was a problem hiding this comment.
Why change toEqual() to toBe()?
cjyuan
added
Complete
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
Sprint-3 is mainly about implementing, rewriting, practicing and and testing functions.
Questions
Ask any questions you have for your reviewer.