Skip to content

London | JAN-2026 ITP | Asha Ahmed | Sprint 3 | implement and rewrite tests coursework #1035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ashaahmed7 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

London | JAN-2026 ITP | Asha Ahmed | Sprint 3 | implement and rewrite tests coursework #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b150432
Implement getAngleType function and added tests for all angle types
ashaahmed7 Feb 21, 2026
bec514f
Implement isProperFraction function and added tests for various fract…
ashaahmed7 Feb 23, 2026
72a8a7d
Implement getCardValue function and added tests for valid and invalid…
ashaahmed7 Feb 23, 2026
5fa7695
Add tests for angle types including right, obtuse, straight, reflex, …
ashaahmed7 Feb 23, 2026
7e4cb27
Added tests for isProperFraction covering various cases including zer…
ashaahmed7 Feb 23, 2026
56c990b
Added tests for getCardValue function covering Aces, Number cards, Fa…
ashaahmed7 Feb 23, 2026
ad8efb8
renamed brach
ashaahmed7 Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion Sprint-3/1-implement-and-rewrite-tests/implement/1-get-angle-type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,29 @@
// execute the code to ensure all tests pass.

function getAngleType(angle) {
// TODO: Implement this function
if (angle > 0 && angle < 90) {
return "Acute angle";
} else if (angle === 90) {
return "Right angle";
} else if (angle > 90 && angle < 180) {
return "Obtuse angle";
} else if (angle === 180) {
return "Straight angle";
} else if (angle > 180 && angle < 360) {
return "Reflex angle";
} else {
return "Invalid angle";
}
Comment on lines +18 to +30
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work here - an opportunity for refactor is considering if every if block contains a return statement do we need else if blocks on lines 20 -26 or can they be just if blocks? Try it out, see if you can predict the outcome and whether the code runs like you expect.

}

console.assert(getAngleType(45) === "Acute angle");
console.assert(getAngleType(90) === "Right angle");
console.assert(getAngleType(120) === "Obtuse angle");
console.assert(getAngleType(180) === "Straight angle");
console.assert(getAngleType(270) === "Reflex angle");
console.assert(getAngleType(0) === "Invalid angle");
console.assert(getAngleType(360) === "Invalid angle");

Comment on lines +33 to +40
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future I'd remove console.asserts as they aren't adding anything to your solution thanks to your actual assertions below on line 60 onwards. Before you submit code make sure you clear those less useful bits like these lines

// The line below allows us to load the getAngleType function into tests in other files.
// This will be useful in the "rewrite tests with jest" step.
module.exports = getAngleType;
Expand All @@ -35,3 +55,27 @@ function assertEquals(actualOutput, targetOutput) {
// Example: Identify Right Angles
const right = getAngleType(90);
assertEquals(right, "Right angle");

// Example: Identify Acute Angles
const acute = getAngleType(45);
assertEquals(acute, "Acute angle");

// Example: Identify Obtuse Angles
const obtuse = getAngleType(120);
assertEquals(obtuse, "Obtuse angle");

// Example: Identify Straight Angles
const straight = getAngleType(180);
assertEquals(straight, "Straight angle");

// Example: Identify Reflex Angles
const reflex = getAngleType(270);
assertEquals(reflex, "Reflex angle");

// Example: Identify Invalid Angles
const invalid1 = getAngleType(0);
assertEquals(invalid1, "Invalid angle");

// Example: Identify Invalid Angles
const invalid2 = getAngleType(360);
assertEquals(invalid2, "Invalid angle");
31 changes: 31 additions & 0 deletions Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@

function isProperFraction(numerator, denominator) {
// TODO: Implement this function
if (denominator <= 0) {
return false;
}
Comment on lines +15 to +17
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You return false on line 21 and you return false here. Can you refactor this function completely to be even more efficient about returning false? Can we return false just once in the solution?

if (numerator < denominator && numerator >= 0) {
return true;
}
Comment on lines +15 to +20
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great example of what i was suggesting in my previous comment! That a plain if block works here, no else if needed. Why is that the case?

return false;
}

// The line below allows us to load the isProperFraction function into tests in other files.
Expand All @@ -31,3 +38,27 @@ function assertEquals(actualOutput, targetOutput) {

// Example: 1/2 is a proper fraction
assertEquals(isProperFraction(1, 2), true);

// Example: 3/5 is a proper fraction
assertEquals(isProperFraction(3, 5), true);

// Example: 1/100 is a proper fraction
assertEquals(isProperFraction(1, 100), true);

// Example: 5/2 is not a proper fraction
assertEquals(isProperFraction(5, 2), false);

// Example: 7/7 is an improper fraction
assertEquals(isProperFraction(7, 7), false);

// Example: 8/3 is an improper fraction
assertEquals(isProperFraction(8, 3), false);

// Example: 5/0 is an invalid fraction
assertEquals(isProperFraction(5, 0), false);

// Example: 2/-6 is an invalid fraction
assertEquals(isProperFraction(2, -6), false);

// Example: -1/4 is an invalid fraction
assertEquals(isProperFraction(-1, 4), false);
44 changes: 44 additions & 0 deletions Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@

function getCardValue(card) {
// TODO: Implement this function
if (!["♠", "♣", "♦", "♥"].includes(card.slice(-1)))
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great use of methods here (includes, slice) and bang operator (!), how did you feel about nesting these methods like this?

throw new Error("Invalid card rank.");
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice throw of an error

const rank = card.slice(0, card.length - 1);
if (rank === "A") return 11;
if (["10", "J", "Q", "K"].includes(rank)) return 10;
if (["2", "3", "4", "5", "6", "7", "8", "9"].includes(rank))
return Number(rank);
Comment on lines +29 to +32
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some super efficient if blocks here, being really accurate and DRY about the return statements.

throw new Error("Invalid card rank.");
}

// The line below allows us to load the getCardValue function into tests in other files.
Expand All @@ -41,6 +49,12 @@ function assertEquals(actualOutput, targetOutput) {
// Examples:
assertEquals(getCardValue("9♠"), 9);

assertEquals(getCardValue("6♥"), 6);

assertEquals(getCardValue("J♣"), 10);

assertEquals(getCardValue("A♦"), 11);

// Handling invalid cards
try {
getCardValue("invalid");
Expand All @@ -50,3 +64,33 @@ try {
} catch (e) {}
Copy link

@netEmmanuel netEmmanuel Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block can be improved. it's currently empty

Copy link
Author

@ashaahmed7 ashaahmed7 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are referring to the original CodeYourFuture code example, which I did not write. The question I was asked was “What other invalid card cases can you think of?” (see below), not to modify the code you are commenting on.


// What other invalid card cases can you think of?

try {
Copy link

@netEmmanuel netEmmanuel Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty catch block is not good practice.

Fix the catch blocks in this file

Copy link
Author

@ashaahmed7 ashaahmed7 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My answers are based on the example given above.

Copy link

@netEmmanuel netEmmanuel Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid empty catch. At the very least, there should be a comment in the empty block explaining why you're swallowing the exception at that point or output the exception.

Look up the standard on how to write a try and catch block; it would help fix the syntax.

Copy link
Author

@ashaahmed7 ashaahmed7 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still learning JavaScript, so I’ll look into catch blocks as I’m not entirely sure what you mean yet. My understanding was that the code is technically correct, and my answers were based on the example provided by CodeYourFuture above. Given that, it feels like a big change to fix the file itself rather than respond to the questions asked.

Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Asha, so Emmanuel is actually right here. We do want that "catch" block to be more informative.

Let's think of it another way. The goal for this test is to show that when an invalid card is passed in, an error is thrown. That's a really interesting problem because when you pass a invalid card into the function, an error will throw and you won't have the chance to "show" that the error behaviour is tested for because the whole programme crashes. Annoying.

But that's why try...catch is so useful. It lets us "try" code in the try... block which might fail, and if an error occurs we can "catch" the error in the catch... block without crashing the programme 🎉

The next step which Emmanuel is referencing is that it isn't enough to just have a empty catch block we should use that block to communicate that the expected behaviour has occurred. In this case, the expected behaviour is the error message is thrown. So that catch block has a parameter e
E.g. catch (e) {} -> can you use this parameter to console log something that SHOWS us that the right error message was thrown?

More helpful reading about catch blocks:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch
https://www.w3schools.com/js/js_errors.asp
https://dev.to/wisdomudo/javascript-trycatch-explained-a-beginners-guide-to-error-handling-2kf6

Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hint: What could that e parameter represent? Docs and logging could help here

Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: this is really a nice-to-have but not an expectation based on how this task is designed. We could argue the console log on line 71 not running is enough evidence in this task that the error was thrown.

getCardValue("9K");

console.log("Error was not thrown for invalid card");
} catch (error) {}

try {
getCardValue("");

console.log("Error was not thrown for invalid card");
} catch (error) {}

try {
getCardValue("ABC");

console.log("Error was not thrown for invalid card");
} catch (error) {}

try {
getCardValue("A");

console.log("Error was not thrown for invalid card");
} catch (error) {}

try {
getCardValue("JK");

console.log("Error was not thrown for invalid card");
} catch (error) {}
Comment on lines +74 to +96
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super repeated code (breaks the Don't Repeat Yourself rule). Could you refactor this code into a reusable function that you pass your invalid cases to?

25 changes: 25 additions & 0 deletions Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/1-get-angle-type.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,32 @@ test(`should return "Acute angle" when (0 < angle < 90)`, () => {
});

// Case 2: Right angle
test(`should return "Right angle" when (angle = 90)`, () => {
expect(getAngleType(90)).toEqual("Right angle");
});

// Case 3: Obtuse angles
test("should identify obtuse angle (90° < angle < 180°)", () => {
expect(getAngleType(91)).toEqual("Obtuse angle");
expect(getAngleType(130)).toEqual("Obtuse angle");
expect(getAngleType(179)).toEqual("Obtuse angle");
});

// Case 4: Straight angle
test("should identify straight angle (angle = 180°)", () => {
expect(getAngleType(180)).toEqual("Straight angle");
});

// Case 5: Reflex angles
test("should identify reflex angle (180° < angle < 360)", () => {
expect(getAngleType(181)).toEqual("Reflex angle");
expect(getAngleType(280)).toEqual("Reflex angle");
expect(getAngleType(359)).toEqual("Reflex angle");
});
Comment on lines +35 to +38
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Across this file, good grouping of similar behaviours.


// Case 6: Invalid angles
test("should identify invalid angle (angle <= 0 or angle >= 360)", () => {
expect(getAngleType(0)).toEqual("Invalid angle");
expect(getAngleType(360)).toEqual("Invalid angle");
expect(getAngleType(361)).toEqual("Invalid angle");
});
64 changes: 63 additions & 1 deletion Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,69 @@ const isProperFraction = require("../implement/2-is-proper-fraction");

// TODO: Write tests in Jest syntax to cover all combinations of positives, negatives, zeros, and other categories.

// Special case: numerator is zero
// Special case: denominator is zero
test(`should return false when denominator is zero`, () => {
expect(isProperFraction(1, 0)).toEqual(false);
});

test("should return true when numerator is zero and denominator is non-zero", () => {
expect(isProperFraction(0, 5)).toEqual(true);
});

test("should return false when both numerator and denominator are zero", () => {
expect(isProperFraction(0, 0)).toEqual(false);
});

// Special case: Identify proper Fractions:
test("should return true for proper fraction", () => {
expect(isProperFraction(2, 3)).toEqual(true);
});

// Special case: Identify Negative Fractions:
test("should return true for proper negative fraction", () => {
expect(isProperFraction(-3, 6)).toEqual(true);
});

test("should return false for improper negative fraction", () => {
expect(isProperFraction(-5, 2)).toEqual(false);
});

test("should return true for proper fraction with negative denominator", () => {
expect(isProperFraction(2, -5)).toEqual(true);
});

test("should return false for improper fraction with negative denominator", () => {
expect(isProperFraction(7, -3)).toEqual(false);
});

// Special case: Identify Equal Numerator and Denominator:
test("should return false for improper fraction (numerator === denominator)", () => {
expect(isProperFraction(3, 3)).toEqual(false);
});

// Special case: Identify both Numerator and Denominator as negative
test("should return true when both numerator and denominator are negative and proper", () => {
expect(isProperFraction(-2, -5)).toEqual(true);
});

test("should return false when both numerator and denominator are negative and improper", () => {
expect(isProperFraction(-6, -3)).toEqual(false);
});

// Special case: Identify both Numerator and Denominator as decimal
test("should return true for proper decimal fractions", () => {
expect(isProperFraction(1.5, 2.5)).toEqual(true);
});

test("should return false for improper decimal fractions", () => {
expect(isProperFraction(2.5, 1.5)).toEqual(false);
});

// Special case: Identify both Numerator and Denominator as large numbers
test("should return true for large proper fractions", () => {
expect(isProperFraction(100, 1000)).toEqual(true);
});

test("should return false for large improper fractions", () => {
expect(isProperFraction(1000, 100)).toEqual(false);
});
69 changes: 68 additions & 1 deletion Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,74 @@ const getCardValue = require("../implement/3-get-card-value");
// Case 1: Ace (A)
test(`Should return 11 when given an ace card`, () => {
expect(getCardValue("A♠")).toEqual(11);
expect(getCardValue("A♣")).toEqual(11);
Copy link

@netEmmanuel netEmmanuel Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are duplicated test lines. Is there a reason for this?

Copy link
Author

@ashaahmed7 ashaahmed7 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a duplication. We have two codes who have different purpose: one for spades and one for clubs. Can you please check again?

Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair to say it is not a duplication here as they are different suites

expect(getCardValue("A♦")).toEqual(11);
expect(getCardValue("A♥")).toEqual(11);
});

// Case 2: Handle Number Cards (2-10):
test("should return the appropriate number from 2 to 10", () => {
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this is an extremely excessive jest test block. I'd prefer to take a snapshot of the test cases that we need to check this behaviour for. Doing every combo of number and suite is very detailed but also quite bulky.
One reason this is a bad idea is it could be computationally heavy, e.g. if the function was running a very complicated algorithm for this behaviour.
But the tradeoff is that you know it works for every case.

I would argue that the "suite" doesn't actually matter in the case described; the number is important. So you should focus on this. If the suite mattered, the test case would be including it in the description but it only mentioned number cards.

expect(getCardValue("2♥")).toEqual(2);
expect(getCardValue("3♥")).toEqual(3);
expect(getCardValue("4♥")).toEqual(4);
expect(getCardValue("5♥")).toEqual(5);
expect(getCardValue("6♥")).toEqual(6);
expect(getCardValue("7♥")).toEqual(7);
expect(getCardValue("8♥")).toEqual(8);
expect(getCardValue("9♥")).toEqual(9);
expect(getCardValue("10♥")).toEqual(10);
expect(getCardValue("2♠")).toEqual(2);
expect(getCardValue("3♠")).toEqual(3);
expect(getCardValue("4♠")).toEqual(4);
expect(getCardValue("5♠")).toEqual(5);
expect(getCardValue("6♠")).toEqual(6);
expect(getCardValue("7♠")).toEqual(7);
expect(getCardValue("8♠")).toEqual(8);
expect(getCardValue("9♠")).toEqual(9);
expect(getCardValue("10♠")).toEqual(10);
expect(getCardValue("2♦")).toEqual(2);
expect(getCardValue("3♦")).toEqual(3);
expect(getCardValue("4♦")).toEqual(4);
expect(getCardValue("5♦")).toEqual(5);
expect(getCardValue("6♦")).toEqual(6);
expect(getCardValue("7♦")).toEqual(7);
expect(getCardValue("8♦")).toEqual(8);
expect(getCardValue("9♦")).toEqual(9);
expect(getCardValue("10♦")).toEqual(10);
expect(getCardValue("2♣")).toEqual(2);
expect(getCardValue("3♣")).toEqual(3);
expect(getCardValue("4♣")).toEqual(4);
expect(getCardValue("5♣")).toEqual(5);
expect(getCardValue("6♣")).toEqual(6);
expect(getCardValue("7♣")).toEqual(7);
expect(getCardValue("8♣")).toEqual(8);
expect(getCardValue("9♣")).toEqual(9);
expect(getCardValue("10♣")).toEqual(10);
});

// Case 3: Handle Face Cards (J, Q, K):
test("should return 10 for face cards", () => {
expect(getCardValue("J♠")).toEqual(10);
expect(getCardValue("Q♠")).toEqual(10);
expect(getCardValue("K♠")).toEqual(10);
expect(getCardValue("J♦")).toEqual(10);
expect(getCardValue("Q♦")).toEqual(10);
expect(getCardValue("K♦")).toEqual(10);
expect(getCardValue("J♣")).toEqual(10);
expect(getCardValue("Q♣")).toEqual(10);
expect(getCardValue("K♣")).toEqual(10);
expect(getCardValue("J♥")).toEqual(10);
expect(getCardValue("Q♥")).toEqual(10);
expect(getCardValue("K♥")).toEqual(10);
});

// Case 4: Handle Invalid Cards:
test("Should return 'Invalid card rank.' for invalid cards", () => {
expect(() => getCardValue("KJ")).toThrow("Invalid card rank.");
expect(() => getCardValue("AK")).toThrow("Invalid card rank.");
expect(() => getCardValue(" ")).toThrow("Invalid card rank.");
expect(() => getCardValue("S♠")).toThrow("Invalid card rank.");
expect(() => getCardValue("J♠♠")).toThrow("Invalid card rank.");
Comment on lines +73 to +77
Copy link
Contributor

@Poonam-raj Poonam-raj Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work

});

// Suggestion: Group the remaining test data into these categories:
Expand All @@ -17,4 +85,3 @@ test(`Should return 11 when given an ace card`, () => {
// To learn how to test whether a function throws an error as expected in Jest,
// please refer to the Jest documentation:
// https://jestjs.io/docs/expect#tothrowerror

Loading