Capetown | May-2025 | Revive Munashe Mapfumo | Coursework/Sprint-3#479
Capetown | May-2025 | Revive Munashe Mapfumo | Coursework/Sprint-3#479rivakutu wants to merge 30 commits intoCodeYourFuture:mainfrom
Conversation
|
To follow best practices, can you update your PR description by carrying out the following actions? |
|
hi. thanks for the feedback. i have checked the checklist and have updated the Changelist. thank you |
| test("should identify Reflex angle", () => { | ||
| expect(getAngleType(181)).toEqual("Reflex angle"); | ||
| }); |
There was a problem hiding this comment.
We could specify multiple expect(...) statements within each test() to cover multiple values that belong to the same case. For example,
test("should identify Reflex angle", () => {
expect(getAngleType(181)).toEqual("Reflex angle");
expect(getAngleType(359.999)).toEqual("Reflex angle");
expect(getAngleType(270)).toEqual("Reflex angle");
});
There was a problem hiding this comment.
I've added more expect statements to reflex angle and obtuse angle tests
| function getCardValue(card) { | ||
| // replace with your code from key-implement | ||
| return 11; | ||
| function getCardValue(rank) { | ||
| if (rank === "A") return 11; | ||
| if (rank === "A♠") return 11; | ||
| if (rank === "2") return 2; | ||
| if (rank === "3") return 3; |
There was a problem hiding this comment.
I think "card" is expected to be a string in the forms "10♠", "K♣", "2♥", "A♦", "13♦". That is, it always ends with a suite character. (Note: The rank of the card "13♦" is invalid).
Can you update your function so that it can extract the rank from the parameter?
There was a problem hiding this comment.
Updated tests to include rank. the function now slices the rank character from the card input before performing computations
| if (rank === "9") return 9; | ||
|
|
||
| if (rank === "10" || rank === "J" || rank === "K" || rank === "Q") return 10; | ||
| else return "Invalid card rank"; |
There was a problem hiding this comment.
The instructions in Sprint-3/1-key-implement/3-get-card-value.js state
"Given a card with an invalid rank ... the function should throw an error indicating "Invalid card rank."
Throwing an error is not the same as returning an error message.
Could you look up "How to throw an error in JS" and update your code accordingly?
There was a problem hiding this comment.
updated the function to throw an error upon invalid card input
| test("should return invalid card rank for unknown cards", () => { | ||
| const InvalidCard = getCardValue("22"); | ||
| expect(InvalidCard).toEqual("Invalid card rank"); | ||
| }); |
There was a problem hiding this comment.
To test if a function can throw an error as expected, we could use .toThrow(). You can find out more about how to use .toThrow() here: https://jestjs.io/docs/expect#tothrowerror (Note: Pay close attention to the syntax of the example)
There was a problem hiding this comment.
updated the function to throw an error upon invalid card input
| // b) What is the if statement used to check | ||
| // when index reaches desired char return it | ||
| // c) Why is index++ being used? | ||
| //idex++ is being used to add 1 to the value of index |
There was a problem hiding this comment.
I think the question is asking, why increment index by one?
There was a problem hiding this comment.
I've answered the question correctly and explained what index++ is used to do
| const password = "12345"; | ||
| // Act | ||
| const result = isValidPassword(password); | ||
| // Assert | ||
| expect(result).toEqual(false); |
There was a problem hiding this comment.
This test does not quite match the description, "password has at least 5 characters".
There was a problem hiding this comment.
added valid tests cases and have incorporated edge cases
| test("password has a number,special symbol, upper and lower case and is 5 char long", () => { | ||
| const password = "ZaR!0"; | ||
| const result = isValidPassword(password); | ||
| expect(result).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
This test checks only if the function can accept a valid password; it does not check if the function can reject invalid passwords such as "ABC4abc", "ABC$abc".
cjyuan
left a comment
There was a problem hiding this comment.
Your changes look good. I only have a few more suggestions.
Well done!
|
|
||
| if (rank === "10" || rank === "J" || rank === "K" || rank === "Q") return 10; | ||
| else return "Invalid card rank"; | ||
| const card = rank.slice(0, -1); // removing last character to get card rank |
There was a problem hiding this comment.
card is the name of the original parameter and is supposed to include both the rank and the suit of a card.
rank is part without the suit.
|
|
||
| test("should return 11 for Ace of Spades", () => { | ||
| const aceofSpades = getCardValue("A♠"); | ||
| expect(aceofSpades).toEqual(11); | ||
| test("should return 11 for Ace of Spade", () => { | ||
| expect(getCardValue("A♠")).toEqual(11); |
There was a problem hiding this comment.
Is this test any different from test at lines 20-21?
There was a problem hiding this comment.
To be more comprehensive, we could set up tests that check if the function can correctly reject a password that fails only one of the following requirements:
- Have at least 5 characters.
- Have at least one English uppercase letter (A-Z) (You have a test the checks this)
- Have at least one English lowercase letter (a-z)
- Have at least one number (0-9)
- Have at least one of the following non-alphanumeric symbols: ("!", "#", "$", "%", ".", "*", "&")
- Must not be any previous password in the passwords array. (You have a test that checks this)
| test("password has a number,special symbol, upper and lower case and is 5 char long", () => { | ||
| // password without upper case letter | ||
| test("password fails, contains no uppercase letter", () => { | ||
| expect(isValidPassword("abcdef!")).toEqual(false); |
There was a problem hiding this comment.
If we were to check if the function can correctly reject a password without an uppercase letter, we should pick a password that meets all other requirements except that it does not contain an uppercase letter.
Because "abcdef!" does not contain any digit or uppercase letter, the function could have returned false because the password does not have a digit.
|
Note: Your forgot to change the label to "Needs review". |

Learners, PR Template
Self checklist
Changelist
Added Jest tests for getAngle, getCardValue, isProperFraction, and other utility functions.
Completed implementations for functions including getOrdinalNumber, repeat, and count.
Validated password functionality with new tests and completed card-validator.
Ensured comprehensive edge case coverage and improved test reliability.
Questions
Ask any questions you have for your reviewer.