Glasgow | 25-ITP-SEP | Alaa Tagi | Sprint 3 | Coursework/sprint 3 implement and rewrite #756

Alaa-Tagi · 2025-10-12T19:48:33Z

Learners, PR Template

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 created a PR with completed tasks as required.

Questions

No questions.

cjyuan

One of the Jest test scripts has not been updated accordingly.

cjyuan · 2025-10-23T11:37:11Z

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

+  if (angle < 90 && angle > 0) {
+    return "Acute angle";
+  }
+  if (angle > 90 && angle < 180) {


Checking angle > 90 is optional because previous if-statements can guarantee angle is always more than 90.

cjyuan · 2025-10-23T11:48:09Z

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

  if (numerator < denominator) {
    return true;
  }
+  if (numerator < 0 && Math.abs(numerator) < denominator) {
+    return false;
+  } 
+  if (numerator >= denominator) {
+    return false;
+  }
+  // we could add more checks here for invalid input, but not required for this exercise


According to the definition of proper fraction in mathematics:

isProperFraction(-4, 3) should return false

isProperFraction(-2, 5) should return true

isProperFraction(-1, 1) should return false

isProperFraction(-2, -3) should return true

Can you look up the definition of proper fraction and update your function accordingly?

cjyuan · 2025-10-23T11:51:31Z

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

+test("should return false for equal numerator and denominator", () => {
+  expect(isProperFraction(3, 3)).toEqual(false);
+});


For each test, we could include multiple test cases that belong to the same category to make the test more complete in the following way:

test("should return false for equal numerator and denominator", () => { expect(isProperFraction(3, 3)).toEqual(false); expect(isProperFraction(-3, 3)).toEqual(false); expect(isProperFraction(3, -3)).toEqual(false); expect(isProperFraction(-3, -3)).toEqual(false); });

cjyuan · 2025-10-23T11:53:45Z

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

+test("should return false for a negative fraction", () => {
+  expect(isProperFraction(-1, 2)).toEqual(true);
+});


The test description does not quite match the specified test.

cjyuan · 2025-10-24T01:52:27Z

Changes look good but not all comments had been addressed.

Expanded test cases for isProperFraction function to include various scenarios such as negative fractions, zero numerator, negative denominator, and large numbers.

Refactor isProperFraction function to include input type checks and handle division by zero. Update logic to determine if a fraction is proper based on the value of the numerator and denominator.

Updated test cases for isProperFraction function to improve clarity and coverage. Added detailed descriptions for each test case.

cjyuan · 2025-10-25T11:17:05Z

There are three files in the "rewrite" folder and you updated only two of them.

Alaa-Tagi · 2025-10-25T17:02:05Z

@cjyuan,
I have face unusal issue. the third file was showing on my VScode but not on my PR. when back to check I had a conflict message. There were unstaged files that's why I had this issue.

cjyuan

I have face unusal issue. the third file was showing on my VScode but not on my PR. when back to check I had a conflict message. There were unstaged files that's why I had this issue.

Is a good opportunity to learn resolving conflict. :)
If you have backed up your files, then the worst that could happen is that you need to submit a new PR.

cjyuan · 2025-10-25T18:32:36Z

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

+test("should return 7 for 7 of Hearts", () => {
+  const sevenOfHearts = getCardValue("7♥");
+  expect(sevenOfHearts).toEqual(7);
+});


When preparing tests, we should ensure the tests cover all possible cases. If we specify a test for individual card, we will need about 53 tests to cover all possible cases. Instead, we could consider classifying all possible values into different categories, and then within each category we test some samples.

For example, one possible category for getCardValue() is, "should return the value of number cards (2-10)", and we can prepare the test as

test("should return the value of number cards (2-10)", () => { expect(getCardValue("2♣︎")).toEqual(2); expect(getCardValue("5♠")).toEqual(5); expect(getCardValue("10♥")).toEqual(10); // Note: We could also use a loop to check all values from 2 to 10. });

cjyuan · 2025-10-25T18:35:24Z

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

+test("should return 10 for Jack of Diamonds", () => {
+  const jackOfDiamonds = getCardValue("J♦");
+  expect(jackOfDiamonds).toEqual(10);
+});
+
+//Case 4
+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);
+});
+


Face cards (J, Q, K) can also be considered a category

The function is not expected to check the suit character. Mentioning the suit character in the test description could send the wrong signal to the person implementing the function.

cjyuan · 2025-10-26T16:27:50Z

Have you double checked what you changed?

cjyuan · 2025-10-26T16:53:50Z

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

+<<<<<<< HEAD
+// Case 2: Improper fraction
+// When numerator > denominator, it should return false.
+test("should return false for an improper fraction", () => {
+=======
+// Case 2: Improper Fraction
+// Given a numerator greater than denominator (5/2),
+// When the function is called,
+// Then it should return false because it's an improper fraction.
+test("should return false for an improper fraction (5/2)", () => {
+>>>>>>> 52740052fb0ad97a2c7253a04978b14a15b9e763


The <<<<<<< HEAD, =======, and >>>>>>> are markers added by Git to show the difference between two versions of a conflicting file.

The difference between the two versions of the conflicting file are indicated by these markers as:

<<<<<<< HEAD

Content from version A

=======

Content from version B

>>>>>>> 52740052fb0ad97a2c7253a04978b14a15b9e763

Note: In your file, the long hexadecimal number following >>>>>>>> is probably the commit SHA of version B.

To resolve the conflict, you will need to manually delete all the unwanted content and the markers in an editor.

@cjyuan, I have delete it

Alaa-Tagi added 6 commits October 9, 2025 19:31

build up getAngleType case by case

b5de08b

Editing is-proper-fraction code

b5fa562

Editing getcardvalue code

3898643

rewrite test with jest for getangletype code

78d374a

update isproperfraction code

d07dd99

Rewrite test with jest for isproperfraction code

a223e45

Alaa-Tagi added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Oct 12, 2025

cjyuan reviewed Oct 23, 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 23, 2025

Alaa-Tagi added 4 commits October 23, 2025 19:59

update the code

17435c8

Editing the code

63246ca

edit the code

b5c55bd

editing the code

a3c8fc8

Alaa-Tagi 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 23, 2025

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 24, 2025

Enhance Jest tests for isProperFraction function

991832e

Expanded test cases for isProperFraction function to include various scenarios such as negative fractions, zero numerator, negative denominator, and large numbers.

Alaa-Tagi 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 24, 2025

Alaa-Tagi added 2 commits October 24, 2025 20:13

Fix the code

f52bd8d

Refactor isProperFraction function to include input type checks and handle division by zero. Update logic to determine if a fraction is proper based on the value of the numerator and denominator.

Editing the jest code

5274005

Updated test cases for isProperFraction function to improve clarity and coverage. Added detailed descriptions for each test case.

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 25, 2025

Editing code

7bc1534

Alaa-Tagi 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 25, 2025

cjyuan reviewed Oct 25, 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 25, 2025

update the jest code

5e63f68

Alaa-Tagi 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 26, 2025

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 26, 2025

cjyuan reviewed Oct 26, 2025

View reviewed changes

manually deletinh all the unwanted content and the markers

27f970f

Alaa-Tagi 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 26, 2025

Uh oh!

Glasgow | 25-ITP-SEP | Alaa Tagi | Sprint 3 | Coursework/sprint 3 implement and rewrite #756

Are you sure you want to change the base?

Glasgow | 25-ITP-SEP | Alaa Tagi | Sprint 3 | Coursework/sprint 3 implement and rewrite #756

Conversation

Alaa-Tagi commented Oct 12, 2025

Learners, PR Template

Changelist

Questions

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 Oct 24, 2025

Uh oh!

cjyuan commented Oct 25, 2025

Uh oh!

Alaa-Tagi commented Oct 25, 2025

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!

cjyuan commented Oct 26, 2025

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!

2 participants