Capetown | May-2025 | Revive Munashe Mapfumo | Coursework/Sprint-3 #479

rivakutu · 2025-06-05T19:46:57Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

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.

cjyuan · 2025-06-16T15:07:35Z

To follow best practices, can you update your PR description by carrying out the following actions?
- Check the items in the Self-Checklist to confirm your pull request meets the guidelines
- Provide a brief description (under the "Changelist" section) summarizing the purpose of the PR and the changes you’ve made

rivakutu · 2025-06-16T15:21:20Z

hi. thanks for the feedback. i have checked the checklist and have updated the Changelist. thank you

cjyuan · 2025-06-16T15:11:12Z

Sprint-3/2-mandatory-rewrite/1-get-angle-type.test.js

+test("should identify Reflex angle", () => {
+  expect(getAngleType(181)).toEqual("Reflex angle");
+});


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"); });

I've added more expect statements to reflex angle and obtuse angle tests

Sprint-3/2-mandatory-rewrite/2-is-proper-fraction.test.js

cjyuan · 2025-06-16T16:11:28Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.js

-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;


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?

Updated tests to include rank. the function now slices the rank character from the card input before performing computations

cjyuan · 2025-06-16T16:28:23Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.js

+  if (rank === "9") return 9;
+
+  if (rank === "10" || rank === "J" || rank === "K" || rank === "Q") return 10;
+  else return "Invalid card rank";


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?

updated the function to throw an error upon invalid card input

cjyuan · 2025-06-16T16:35:10Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js

+test("should return invalid card rank for unknown cards", () => {
+  const InvalidCard = getCardValue("22");
+  expect(InvalidCard).toEqual("Invalid card rank");
+});


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)

updated the function to throw an error upon invalid card input

Sprint-3/4-stretch-investigate/card-validator.js

cjyuan · 2025-06-16T18:02:31Z

Sprint-3/4-stretch-investigate/find.js

 // 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


I think the question is asking, why increment index by one?

I've answered the question correctly and explained what index++ is used to do

Sprint-3/4-stretch-investigate/password-validator.js

cjyuan · 2025-06-16T18:19:34Z

Sprint-3/4-stretch-investigate/password-validator.test.js

+  const password = "12345";
+  // Act
+  const result = isValidPassword(password);
+  // Assert
+  expect(result).toEqual(false);


This test does not quite match the description, "password has at least 5 characters".

added valid tests cases and have incorporated edge cases

cjyuan · 2025-06-16T18:25:54Z

Sprint-3/4-stretch-investigate/password-validator.test.js

+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);
+});


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".

… function well

cjyuan

Your changes look good. I only have a few more suggestions.

Well done!

cjyuan · 2025-06-17T23:10:16Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.js


-  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


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.

cjyuan · 2025-06-17T23:13:08Z

Sprint-3/2-mandatory-rewrite/3-get-card-value.test.js


-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);


Is this test any different from test at lines 20-21?

cjyuan · 2025-06-17T23:24:04Z

Sprint-3/4-stretch-investigate/password-validator.test.js

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)

cjyuan · 2025-06-17T23:27:32Z

Sprint-3/4-stretch-investigate/password-validator.test.js

-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);


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.

cjyuan · 2025-06-17T23:28:54Z

Note: Your forgot to change the label to "Needs review".

rivakutu added 20 commits June 5, 2025 16:49

applied angle tests

9372367

added tests

a4fa235

get card

ea3b833

get angle completion

160782d

get angle tests

03d4d32

is proper fraction complete

58113a5

is proper fraction test complete

02cc899

updated

d8c6e50

get-card tests completed

8541c1b

get card value completed

300f27a

count completed

234601f

count tests completed

4c09bc3

get ordinal number completed

2a6afb4

get ordinal number test completed

ac4e3c5

repeat completed

2f40b44

repeat test completed

a7f8489

card-validator implementation

4551545

find questoions answered

f5770e7

password validator completed

bd8522b

password validator tests

c166bd1

rivakutu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 5, 2025

rivakutu mentioned this pull request Jun 7, 2025

Revive Mapfumo Pull requests Coursework/Sprint 1-3 rivakutu/My-Coursework-Planner#23

Open

cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jun 16, 2025

cjyuan reviewed Jun 16, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jun 16, 2025

rivakutu added 10 commits June 17, 2025 14:18

added more expect statements to reflex angle and obtuse angle tests

facd23d

made the function more simpler but robust

e77d856

minor if statement appearing adjustment

badfd94

updated the test cases to mathematical correctness

09ed00d

fixed variable mismatch

959cbed

corrected test cases

6130cd8

updated function names to start with lower case

3b23fd2

explaining the importance of index++

934f970

function names updated to match requirements

d70bd76

added more robust testing. includes more edges cases and utilizes the…

a85bce4

… function well

cjyuan reviewed Jun 17, 2025

View reviewed changes

cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jun 17, 2025

illicitonion closed this Sep 1, 2025

Uh oh!

Capetown | May-2025 | Revive Munashe Mapfumo | Coursework/Sprint-3 #479

Capetown | May-2025 | Revive Munashe Mapfumo | Coursework/Sprint-3 #479

Uh oh!

Conversation

rivakutu commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Questions

Uh oh!

cjyuan commented Jun 16, 2025

Uh oh!

rivakutu commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjyuan commented Jun 17, 2025

Uh oh!

Uh oh!

rivakutu commented Jun 5, 2025 •

edited

Loading

rivakutu commented Jun 16, 2025 •

edited

Loading