Birmingham | 25-ITP-Sep | Ahmad Ehsas | Sprint 3 | Implement-and-rewrite-tests #735

ahmadehsas · 2025-10-05T22:00:45Z

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist:

I have completed all the mandatory tasks. The changes I have made in the files are as below:

Sprint-3/1-key-implement
Sprint-3/2-mandatory-rewrite
Sprint-3/3-mandatory-practice

Questions

…feedback

…, and invalid cards

…he goal of this function is to count how many times a specific character appears in a given string

…successfully.

…xpected error.

…ositive integer.

…lues and update the test case.

github-actions · 2025-10-05T22:00:57Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

ahmadehsas · 2025-10-30T17:27:30Z

Thank you for your reviewing. I just have a question, should I make another branch for ‘practice-tdd’ backlog or do that in the same branch?

…

On Thu, 30 Oct 2025 at 15:58, CJ Yuan ***@***.***> wrote: *cjyuan* left a comment (CodeYourFuture/Module-Structuring-and-Testing-Data#735) <#735 (comment)> Good job in keeping the branch updated. Now let's focus on keeping the branch clean. On the upstream branch (CYF's main), the subfolders inside "Sprint-3" folder have been renamed. As a result, the filenames are all "messed up", and in short, that makes code reviewing difficult. Can you transfer all of your changes to the files in these three subfolders, and then remove all other subfolders in "Sprint-3" folder? - Sprint-3/1-implement-and-rewrite-tests - Sprint-3/2-practice-tdd - Sprint-3/3-stretch — Reply to this email directly, view it on GitHub <#735 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BOYJJVD77WICLBREAB6UM2L32IYRHAVCNFSM6AAAAACILF2MFGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINRYG42DMMJQGI> . You are receiving this because you were mentioned.Message ID: <CodeYourFuture/Module-Structuring-and-Testing-Data/pull/735/c3468746102@ github.com>

…ther subfolders.

cjyuan · 2025-10-31T13:28:29Z

Thank you for your reviewing. I just have a question, should I make another
branch for ‘practice-tdd’ backlog or do that in the same branch?

Either way works.

cjyuan

Good job in fixing the branch.

Sprint-3/1-implement-and-rewrite-tests/implement/1-get-angle-type.js

cjyuan · 2025-10-31T13:35:15Z

Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js

+  if (Math.abs(numerator) < denominator) return true; // This version of code works correctly for proper and negative fractions.
+  if (Math.abs(numerator) >= Math.abs(denominator)) return false;
+  if (Math.abs(numerator) === Math.abs(denominator)) return false;
+
  if (numerator < denominator) {
    return true;
  }


What do you expect from isProperFraction(-2, -3)?

Some of the code is redundant.

My expectation for 'isProperFraction (-2, -3) ': the fraction is proper because the absolute value of -2 / -3 is 2 / 3 and the numerator is less than the denominator, so the return should be true.

The redundant code has been removed.

cjyuan · 2025-10-31T13:41:03Z

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

+  if (!isNaN(rank)) {
+    return Number(rank); // Number card
+  }


In JavaScript, strings that represent valid numeric literals in the language can be safely converted to equivalent numbers. For examples, "0x02", "2.1", "9e1" or "0002"

Do you want to recognize these string values as valid ranks?

To recognise string values as valid ranks, I used 'Number(rank)'
console.log(Number("0x002"));
console.log(Number("2.1"));
console.log(Number("9e1"));
console.log(Number("0002"));

cjyuan · 2025-10-31T13:44:49Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js

+test("should return true for negative fractions", () => {
+  const negativeFraction = isProperFraction(-4, 7);
+  expect(negativeFraction).toEqual(true);
+});
+


Not all negative fractions are proper fractions. For example, -5/2.

If you allow negative denominator, then you could also test isProperFraction(4, -7), isProperFraction(-4, -7), isProperFraction(-7, 4), isProperFraction(-7, -4), isProperFraction(7, -4).

I employed 'Math.abs(denominator)' to handle fractions with negative denominators.

The function is correct. I was referring to test and the test description.
The test description, "should return true for negative fractions", is misleading because negative fractions can be proper or improper.

cjyuan · 2025-10-31T13:54:27Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js

+test("throws an error or invalid card", () => {
+  expect(() => getCardValue("Z♠")).toThrow("Invalid card");
+});
+
+test("should throw error for malformed numeric input", () => {
+  expect(() => getCardValue("2.9999♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for repeated characters in rank", () => {
+  expect(() => getCardValue("3AAAA♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for number beyond valid range", () => {
+  expect(() => getCardValue("11♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for missing suit", () => {
+  expect(() => getCardValue("Q")).toThrow("Invalid card rank");
+});
+
+test("should throw error for non-string input", () => {
+  expect(() => getCardValue(5)).toThrow("Card must be a string");
+});
+


Can you current implementation pass all these tests?

Why specifically test "repeated characters" in rank? What's so special about this particular string pattern?

"2.9999" is not really a malformed numeric value.

The function is not expected to check the suit character. Including a test for missing suit might send the wrong message to the person implementing the function.

Why specifically test "repeated characters" in rank? What's so special about this particular string pattern?

test("should throw error for repeated characters in rank", () => { expect(() => getCardValue("3AAAA♠")).toThrow("Invalid card rank"); });

cjyuan · 2025-10-31T14:02:38Z

Sprint-1/Sprint-1/1-key-exercises/1-count.js

The subfolder containing these two files were moved from its original folder.

…enominator.

cjyuan · 2025-11-05T00:16:13Z

Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js

+  if (Math.abs(numerator) < Math.abs(denominator)) return true; // This version of code works correctly for proper and negative fractions.
+  if (Math.abs(numerator) >= Math.abs(denominator)) return false;
+  if (Math.abs(numerator) === Math.abs(denominator)) return false;


Some of these code will never get executed.

If the condition of the if statements are opposite of each other, we could use if-else.

cjyuan · 2025-11-05T00:18:54Z

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

+  if (!/^[0-9]+$/.test(rank)) {
+    throw new Error("Invalid card rank");
+  }


This is a stricter check. Still, any string containing only digits could pass this test. For example, "0000002".

cjyuan · 2025-11-05T00:24:28Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js

+test("should return true for negative fractions", () => {
+  const negativeFraction = isProperFraction(-4, 7);
+  expect(negativeFraction).toEqual(true);
+});
+


The function is correct. I was referring to test and the test description.
The test description, "should return true for negative fractions", is misleading because negative fractions can be proper or improper.

cjyuan · 2025-11-05T00:27:15Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js

+test("throws an error or invalid card", () => {
+  expect(() => getCardValue("Z♠")).toThrow("Invalid card");
+});
+
+test("should throw error for malformed numeric input", () => {
+  expect(() => getCardValue("2.9999♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for repeated characters in rank", () => {
+  expect(() => getCardValue("3AAAA♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for number beyond valid range", () => {
+  expect(() => getCardValue("11♠")).toThrow("Invalid card rank");
+});
+
+test("should throw error for missing suit", () => {
+  expect(() => getCardValue("Q")).toThrow("Invalid card rank");
+});
+
+test("should throw error for non-string input", () => {
+  expect(() => getCardValue(5)).toThrow("Card must be a string");
+});
+


Why specifically test "repeated characters" in rank? What's so special about this particular string pattern?

test("should throw error for repeated characters in rank", () => { expect(() => getCardValue("3AAAA♠")).toThrow("Invalid card rank"); });

cjyuan · 2025-11-06T04:59:35Z

Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

+
+
+  // Anything else is invalid
+   throw new Error("Invalid card rank")


Can you figure out why this statement will never get executed?

cjyuan · 2025-11-06T05:06:49Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js

 // Case 3: Identify Negative Fractions:
+test("should return true when the absolute value of numerator is less than denominator, even if the fraction is negative", () => {
+  const negativeFraction = isProperFraction(-4, 7);
+  expect(negativeFraction).toEqual(true);
+});
+


Instead of just testing one particular form of negative fraction, we could also test (under the same category)

isProperFraction(4, -7); isProperFraction(-4, -7); isProperFraction(7, -4); ...

to make the test more complete.

cjyuan · 2025-11-06T05:08:11Z

Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js

+try {
+getCardValue("Z♠");
+} catch (error) {
+console.log("Caught error:", error.message);
+}
+


Debugging code?

ahmadehsas added 29 commits June 16, 2025 17:22

Answered comment for count update in exercise -1

65fe894

Implement getAngleType with complete test coverage and improved test …

532fd66

…feedback

Completed the rest the tests and cases

0cba6bb

Implement getCardValue function with test cases for number, face, ace…

8d6eb95

…, and invalid cards

Implement getAngleType function to classify angle types

2479c4f

Add test cases for angle types and update Sprint-1 readme

3fafedf

Added function(isProperFraction)

7a5fe08

Identified Improper, Negative and equal fractions.

68f5551

Rewrote the Card value function and tests

5dd3b88

completed the countChar function with a short clear description and t…

62ef41d

…he goal of this function is to count how many times a specific character appears in a given string

completed the No occurrences Scenario test

1443cbf

completed the function getOrdinalNumber.

1e2318b

the test is passed for get-ordinal-number.js

edea881

Implemented the function and the test was passed

dbf4e92

Edited incorrect test cases and fixed logic to ensure all tests pass …

fad2100

…successfully.

Add else condition to simplify my code.

1154f89

Add jest .toThrow method for checking if the function can throw the e…

047df5d

…xpected error.

implement the the function getOrdinalNumber that works for any vali p…

ba0f73e

…ositive integer.

prepare tests for all valid integers.

5aeb5c0

combine tests for number cards and Face cards.

550f44d

update the function to hundle invalid input.

e76a59c

update the test code to hundle expected cases.

3415822

Match the error message in toThrow() matcher function.

bf5f7f8

Delete Sprint-1/1-key-exercises/1-count.js

4f1e86a

Delete Sprint-1/readme.md

e5699a5

Restored deleted files from Sprint-1 to fix accidental changes

63d4861

update the tests for all valid integers.

e749f57

Add test code to cover all valid integers.

d34b464

Add input validation to ensure the function only accept valid card va…

fb90283

…lues and update the test case.

cjyuan added the Reviewed Volunteer to add when completing a review with trainee action still to take. label Oct 30, 2025

Transferred all of my changes to the updated foldersand removed all o…

30dfbd8

…ther subfolders.

ahmadehsas added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 30, 2025

cjyuan reviewed Oct 31, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 31, 2025

ahmadehsas added 5 commits November 2, 2025 18:08

Removed redundent code.

a0cf3ae

Removed redundant code and add condition for negative nemurator and d…

56ccaef

…enominator.

recognized string values as valid ranks.

5c8aac3

The test is passed for negative denominator fraction.

8accc29

The implementation of all tests has been passed.

5b0a96e

ahmadehsas added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 3, 2025

Save updates to proper-fraction and card-value functions

3b28213

cjyuan reviewed Nov 5, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 5, 2025

ahmadehsas added 4 commits November 5, 2025 18:24

Updated my code.

632c967

Updated the description of Case 3: Identify Negative fraction.

8ba9480

Removed redundancy test case and update the test cases.

a25de1f

Updated the code in line 43 & 44 of get card value.

09cf05f

ahmadehsas added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 5, 2025

cjyuan reviewed Nov 6, 2025

View reviewed changes

cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 6, 2025

ahmadehsas mentioned this pull request Nov 6, 2025

Birmingham | Structuring and Testing Data | Ahmad Ehsas ahmadehsas/My-Coursework-Planner#82

Open



		// Anything else is invalid
		throw new Error("Invalid card rank")

Uh oh!

Birmingham | 25-ITP-Sep | Ahmad Ehsas | Sprint 3 | Implement-and-rewrite-tests #735

Are you sure you want to change the base?

Birmingham | 25-ITP-Sep | Ahmad Ehsas | Sprint 3 | Implement-and-rewrite-tests #735

Uh oh!

Conversation

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

Uh oh!

Changelist:

Questions

Uh oh!

github-actions bot commented Oct 5, 2025

Uh oh!

ahmadehsas commented Oct 30, 2025 via email

Uh oh!

cjyuan commented Oct 31, 2025

Uh oh!

cjyuan left a comment

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ahmadehsas commented Oct 5, 2025 •

edited

Loading