Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 3 | Coursework/1-implement-and-rewrite-tests #745

M-Abdoon · 2025-10-10T15:51:58Z

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 implemented and rewrote tests, figured the errors and tested the code. modified the files to meet the test requirements.

Questions

No questions for now, thank you.

cjyuan

The files in 2-practice-tdd subfolder are not related to this exercise. Can you revert the changes made in that folder to keep this branch clean?
Can you rename this branch to the name specified in the backlog?

cjyuan · 2025-10-17T19:48:19Z

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

+  if(typeof(numerator) != "number" || typeof(denominator) != "number") {
+    return "[ wrong input ]";
+  }
+  if (numerator < denominator ) {
    return true;
  }
+  return false;
 }


In mathematics, -A/B == A/-B == -(A/B), and -A/-B == A/B for any integers A and B (B ≠ 0).
They represent a proper fraction if A < B and B ≠ 0.

So isProperFraction(-4, 3) should return false because 4 >= 3.

So isProperFraction(-2, 5) should return true because 2 < 5.

Consider comparing the absolute value of the numerator and the denominator instead.

In addition, to indicate errors, it is best practices to throw an error (instead of returning an error message).
If you have time, you should check out how to throw an error in JS.

Alternatively, you can choose to return false to indicate something is not a fraction.

I made the required changes, I used the method Math.abs() to fix the bug and used error throwing when inputs are not valid. also the function should return 0 when denominator is 0;

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

cjyuan · 2025-10-17T19:51:15Z

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

+  if ((rank => 2) && (rank <= 10)) {
+    return Number(rank);
+  }


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

Does your function return the value you expected from each of the following function calls?

getCardValue("0x02♠"); getCardValue("2.1♠"); getCardValue("0002♠");

Thank you for pointing that out. Yes, the function returns what is expected, but I used Math.floor() to exclude decimal numbers.

cjyuan · 2025-10-17T19:53:40Z

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

+test("should identify Reflex angle (greater than 180° and less than 360°)", () => {
+  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 (180 < angle < 360)", () => { expect(getAngleType(300)).toEqual("Reflex angle"); expect(getAngleType(359.999)).toEqual("Reflex angle"); expect(getAngleType(180.001)).toEqual("Reflex angle"); });

cjyuan · 2025-10-17T19:56:52Z

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

+test("should return true for a negative Fractions", () => {
+  expect(isProperFraction(-6, 4)).toEqual(true);
+});


-6/4 is not a proper fraction.

You can test several "negative fractions in one test". For examples,

test("should return true for a negative Fractions", () => { expect(isProperFraction(-6, 4)).toEqual(false); expect(isProperFraction(-4, 6)).toEqual(true); expect(isProperFraction(6, -4)).toEqual(false); expect(isProperFraction(4, -6)).toEqual(true); expect(isProperFraction(-6, -4)).toEqual(false); expect(isProperFraction(-4, -6)).toEqual(true); });

Done. I added more test cases to each category.

cjyuan · 2025-10-17T19:57:58Z

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

+test("should return the number entered as input", () => {
+  const numberCards = getCardValue("4♠");
+  expect(numberCards).toEqual(4);
+});
+


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

Ok. I changed the code and added more test cases + added different statements using for loop to test numbers from 2 to 10

M-Abdoon · 2025-10-19T13:14:30Z

The files in 2-practice-tdd subfolder are not related to this exercise. Can you revert the changes made in that folder to keep this branch clean?

Can you rename this branch to the name specified in the backlog?

Ok, I will delete the unnecessery folder, thank you.
yes I can change the branch name, but if I do that, the pull request will be closed and I will have to make another pull request from the new branch, should I do that?
in the next pull request I`ll make sure that the branch is named as requested in the backlog.

cjyuan · 2025-10-19T14:50:22Z

We can rename the remote branch directly on GitHub and keep the PR stay opened.
However, we would also need to update our local branch to track the new remote name. (You will have to run some Git command on CLI). (Ask AI how to accomplish this).

Go ahead and backup your files and try. This is a learning experience and there won't be any penalty.

M-Abdoon · 2025-10-20T12:46:30Z

We can rename the remote branch directly on GitHub and keep the PR stay opened. However, we would also need to update our local branch to track the new remote name. (You will have to run some Git command on CLI). (Ask AI how to accomplish this).

Go ahead and backup your files and try. This is a learning experience and there won't be any penalty.

It didn’t work. I tried renaming it directly on GitHub and also tried renaming it locally and pushing it, but the PR still gets closed. I even asked different AI tools, ChatGPT, Copilot, and Gemini, and they all ended up saying that if I want to keep my PR, I just don’t rename it! I guess I’ll need extra help; maybe I’ll ask a volunteer to help me during the workshop next Saturday.

…results

cjyuan · 2025-10-20T16:26:00Z

You are right. The PR will be closed when the branch is renamed. I remembered incorrectly. Sorry about that. You can just keep this branch and PR (instead of opening a new one).

…erent case tests for the getCardValue function - looped inputs too

M-Abdoon · 2025-10-21T01:29:44Z

No worries at all, I learned several things while trying to rename the branch without having it closed :)

The required changes have been made, thank you.

cjyuan

Changes look good. I have few more suggestions but I will go ahead and mark this PR as complete first as it has met all the basic requirements.

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

cjyuan · 2025-10-21T10:38:02Z

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

+  rank = Number(Math.floor(rank));
+
+  if ((rank >= 2) && (rank <= 10)) {
+    return rank;
+  }


This approach ensures values like "3.1416" or "0x00002" get returned as an equivalent integer, but do you want to consider "3.1416♥" or "0x00002♠" valid card values?

I want to exclude them, so I added this condition :

if(!Number.isInteger(Number(rank) ) || /^0x/i.test(rank)) { return "Invalid card rank."; }

to ensure that the number is an integer without a decimal part, and to make sure it isn’t a hexadecimal value by using a regex with test().

If you are using this approach (to consider what to reject), you would need to consider all valid numeric literal representations in the JS language. You can look up how numbers can be represented in the JS language.

Instead of focusing on what values are invalid, we could focus on what values are valid.

Ok, I understand.
Thank you so much.

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

…licing it

M-Abdoon · 2025-10-21T13:04:41Z

Changes look good. I have few more suggestions but I will go ahead and mark this PR as complete first as it has met all the basic requirements.

Thank you so much. I applied your suggestions to my code and gained better knowledge from them. Thanks again :)

M-Abdoon added 8 commits October 6, 2025 17:31

to ignore testing.js file

aa5dc2a

get angle type exercise done

a805d98

2 proper fraction exercise done

4b4d8da

get card value exercise done

ba6d069

rewrite get angle type exercise done

79156df

rewrite: is proper fraction exersice done

049c628

rewrite: get card value test exercise done

6cd7b35

count.js + cout.test.js modified to meet requirements

d489ca1

M-Abdoon added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 10, 2025

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

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

M-Abdoon closed this Oct 17, 2025

M-Abdoon deleted the Sprint-3/1_implement_and_rewrite_tests branch October 17, 2025 21:32

M-Abdoon restored the Sprint-3/1_implement_and_rewrite_tests branch October 17, 2025 21:33

M-Abdoon reopened this Oct 17, 2025

M-Abdoon closed this Oct 20, 2025

M-Abdoon deleted the Sprint-3/1_implement_and_rewrite_tests branch October 20, 2025 11:59

M-Abdoon restored the Sprint-3/1_implement_and_rewrite_tests branch October 20, 2025 12:07

M-Abdoon reopened this Oct 20, 2025

M-Abdoon changed the base branch from main to cjyuan-patch-1 October 20, 2025 12:38

M-Abdoon changed the base branch from cjyuan-patch-1 to main October 20, 2025 12:38

M-Abdoon closed this Oct 20, 2025

M-Abdoon deleted the Sprint-3/1_implement_and_rewrite_tests branch October 20, 2025 12:39

M-Abdoon restored the Sprint-3/1_implement_and_rewrite_tests branch October 20, 2025 12:46

M-Abdoon reopened this Oct 20, 2025

M-Abdoon added 2 commits October 20, 2025 16:30

Modified the function isProperFraction() for beter and more accurate …

123c3c2

…results

updated getCardValue() function

7a3bb54

M-Abdoon added 4 commits October 21, 2025 00:50

Revert unwanted folder 2-practice-tdd

4e2d6be

added more test cases, specified multiple statements withing each test()

7e35093

added more test cases withing each test()

f8f268c

Edited a rypo withing the , >= instead of =>. and added more and diff…

35475f7

…erent case tests for the getCardValue function - looped inputs too

M-Abdoon 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 21, 2025

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

M-Abdoon added 6 commits October 21, 2025 12:54

changed if conditions to make them accurate

76a034c

return result in only one line

effa3b4

Delete unwanted strange code

8237ab8

prepare function to deal with number inputs by adding String() when s…

2214586

…licing it

treat decimal numbers and hex values as invalid inputs

8e2e5eb

remove unnecesarry test cases

37aedba

Uh oh!

Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 3 | Coursework/1-implement-and-rewrite-tests #745

Are you sure you want to change the base?

Glasgow | 25-ITP-SEP | Mohammed Abdoon | Sprint 3 | Coursework/1-implement-and-rewrite-tests #745

Uh oh!

Conversation

M-Abdoon commented Oct 10, 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!

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!

M-Abdoon commented Oct 19, 2025

Uh oh!

cjyuan commented Oct 19, 2025

Uh oh!

M-Abdoon commented Oct 20, 2025

Uh oh!

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

Uh oh!

Uh oh!

M-Abdoon commented Oct 21, 2025

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

M-Abdoon commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cjyuan commented Oct 20, 2025 •

edited

Loading

cjyuan Oct 21, 2025 •

edited

Loading