Glasgow | 25-ITP-Sep | Fares Bakhet | Sprint 3 | coursework/sprint-3-implement-and-rewrite #755

Fares-Bakhet · 2025-10-12T19:46:34Z

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

-Implementing cases and tests for each task.
-Test all the files using 'Jest'.

Questions

No questions.

cjyuan

Code is good. I just have a few suggestions.

cjyuan · 2025-10-23T11:21:08Z

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, Thanks for that. I have removed the unnecessary part.

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

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);
+});
+test("should return 10 for Queen of Clubs", () => {
+  const queenofClubs = getCardValue("Q♣");
+  expect(queenofClubs).toEqual(10);
+});
+test("should return 10 for King of Hearts", () => {
+  const kingofHearts = getCardValue("K♥");
+  expect(kingofHearts).toEqual(10);
+});


We could generalise these tests into one test to "should return 10 for face cards (J, Q, K)" and check all three ranks J, Q, K).

In addition, the function is not expected to check t the suit character. Indicating the suit character in the test description may send the developer the wrong message.

@cjyuan, I have generalised the test. Could you explain what you mean by check t?

Just a typo. Should be just "check the ..."

cjyuan · 2025-10-23T11:33:07Z

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

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


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, Do I need to redo my tests again in a better way? Or is it just for future knowledge?😅😅

Why not practice it now?

cjyuan · 2025-10-23T14:32:04Z

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

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


Can you find all places where you can remove redundant checks and update the code accordingly?

@cjyuan, okay.

@cjyuan,
If I want to rearrange this again, I will end up in this way:

0 & < 90 >> covers 1-89

==90 >> covers 90

90 & <180 >>covers 91-179

==180>>covers 180

180 & <360 >> covers 181-359

invalid if sth else

Right??

I can't tell. 180 & < 360 is not exactly JS code.

@cjyuan,
I was shortcutting
if( angle > 180 && angle <360)

I still don't know wat you try to expressed:

I suspect part of the code is misinterpreted as Markdown code. Here is the GitHub Markdown guide:
https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

cjyuan · 2025-10-23T14:32:50Z

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

+test("should return 10 for all face cards (J, Q, K)", () => {
+  const faceCards = ["J♦", "Q♣", "K♥"];
+  faceCards.forEach((card) => {
+    const value = getCardValue(card);
+    expect(value).toEqual(10);
+  });
 });


This looks great.

cjyuan · 2025-10-23T21:20:06Z

The updated Jest tests looks good.

Fares-Bakhet added 6 commits October 12, 2025 17:56

Editing the getAngleType Code

8738ad1

Editing isProoerFraction Code

ed40750

Editing getCardValue Code

e6d384c

Passed the test for getAngleType code

a2b2e06

Passed getCardvalue test

11efcb1

Passed isProperFraction test

d440aaf

Fares-Bakhet 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

Fares-Bakhet added 3 commits October 23, 2025 12:55

Deleted angle>90

07685f8

Editing the code

283fe06

Generalise test case3

7139e24

Fares-Bakhet 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 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

Fares-Bakhet added 2 commits October 23, 2025 16:42

Editing

29b1e29

Re-write the tests

4c6b2b2

Fares-Bakhet 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 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 Oct 23, 2025

Fares-Bakhet mentioned this pull request Oct 28, 2025

25-SEP-ITP| Structuring and testing data | Fares Bakhet Fares-Bakhet/My-Coursework-Planner#57

Open

Uh oh!

Glasgow | 25-ITP-Sep | Fares Bakhet | Sprint 3 | coursework/sprint-3-implement-and-rewrite #755

Are you sure you want to change the base?

Glasgow | 25-ITP-Sep | Fares Bakhet | Sprint 3 | coursework/sprint-3-implement-and-rewrite #755

Uh oh!

Conversation

Fares-Bakhet 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!

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!

cjyuan commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants