Repository navigation
London | 26-ITP-Sep | Diana Ausiejute | Sprint 2 |Implement and Rewrite Tests #1672
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
698f2ba
898be59
7eba76c
6033efe
5ec0fc9
a7a2c63
77f784c
29c23e6
12da92f
7e9f694
55184b4
8531a8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,21 @@ | |
| // After you have implemented the function, write tests to cover all the cases, and | ||
| // execute the code to ensure all tests pass. | ||
|
|
||
| export function getAngleType(angle) { | ||
| // TODO: Implement this function | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if someone passes a non-number like "90" or null here? Should we add validation
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I did not take that into consideration. Passing a regular string would return an "invalid angle", but an empty one or null would not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great! changes implemented |
||
| function getAngleType(angle) { | ||
| if (isNaN(angle) || angle == null || angle === "") return `Invalid Input`; | ||
|
|
||
| if (angle === 90) { | ||
| return `Right angle`; | ||
| } else if (angle >= 0 && angle < 90) { | ||
| return `Acute 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`; | ||
| } | ||
| return `Out of range`; | ||
| } | ||
|
|
||
| export { getAngleType }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ | |
| // After you have implemented the function, write tests to cover all the cases, and | ||
| // execute the code to ensure all tests pass. | ||
|
|
||
| export function isProperFraction(numerator, denominator) { | ||
| // TODO: Implement this function | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this function is declared twice here. What do you think happens to that inner function since it's never called
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the function declaration/export style slightly and if you view the file it's more clear that the function has been declared once There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah!, great catch |
||
| function isProperFraction(numerator, denominator) { | ||
| return Math.abs(numerator) < Math.abs(denominator); | ||
| } | ||
|
|
||
| export { isProperFraction }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,38 @@ | |
| // After you have implemented the function, write tests to cover all the cases, and | ||
| // execute the code to ensure all tests pass. | ||
|
|
||
| export function getCardValue(card) { | ||
| // TODO: Implement this function | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just like the other files, this function is declared twice. What happens to the inner function since it isn't exported or called
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once again, the function is declared once.. The export statement simply makes that existing function available to other modules; it does not declare another function or create an inner function. Your comments are written on the deleted sections There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out! Just like in the other exercise, this was down to how I was viewing the diff in GitHub (Unified View showing the old red lines alongside the new green lines). Your code actually only declares the function once and is completely fine. Great job on the implementation! |
||
| function getCardValue(card) { | ||
| if (card == "") { | ||
| throw new Error(`Missing a specific number and specific suit symbol`); | ||
| } | ||
| const cardRank = card.slice(0, -1); | ||
| const suit = card.slice(-1); | ||
|
|
||
| if (!["♠", "♥", "♦", "♣"].includes(suit)) { | ||
| throw new Error(`Missing a specific suit symbol`); | ||
| } | ||
|
|
||
| if (cardRank == "A") { | ||
| return 11; | ||
| } | ||
| if (cardRank == "J" || cardRank == "K" || cardRank == "Q") { | ||
| return 10; | ||
| } | ||
| if ( | ||
| cardRank == "2" || | ||
| cardRank == "3" || | ||
| cardRank == "4" || | ||
| cardRank == "5" || | ||
| cardRank == "6" || | ||
| cardRank == "7" || | ||
| cardRank == "8" || | ||
| cardRank == "9" || | ||
| cardRank == "10" | ||
| ) { | ||
| return Number(cardRank); | ||
| } else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The instructions above mention that an invalid card string should throw an error. How does returning the string "invalid" compare to throwing an error
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. returning the string "invalid" would still allow the program to continue running without interruption, whereas throwing an error indicates a more serious issue that requires immediate attention and handling Edit: I see now that some misunderstandings happened due to us looking at different versions of the file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spot on! You've captured the exact difference in control flow—returning a value allows execution to continue silently, whereas throwing an error halts execution and forces the caller to handle the edge case explicitly. Also, completely agree regarding the diff mix-up thanks for bearing with me while I sorted out my view settings! |
||
| throw new Error(`Missing a specific number and specific suit symbol`); | ||
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any debug logging at the bottom of the file that we should remove before finalizing?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry I couldn't find any There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check other comments about this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any debug logging (console.log) at the bottom of the file that we should remove before finalizing or committing our code?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all sorted now, spot on! |
||
| export { getCardValue }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,52 @@ test(`Should return 11 when given an ace card`, () => { | |
| // Number Cards (2-10) | ||
| // Face Cards (J, Q, K) | ||
| // Invalid Cards | ||
|
|
||
| // 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 | ||
| // -----number cards------ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GREAT WORK HERE!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! it's great to know how to avoid repeating the code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice Refactor with test.each |
||
| test.each([ | ||
| { card: "2♥", value: 2 }, | ||
| { card: "3♣", value: 3 }, | ||
| { card: "4♦", value: 4 }, | ||
| { card: "5♠", value: 5 }, | ||
| { card: "6♦", value: 6 }, | ||
| { card: "7♥", value: 7 }, | ||
| { card: "8♠", value: 8 }, | ||
| { card: "9♥", value: 9 }, | ||
| { card: "10♣", value: 10 }, | ||
| ])("Should return $value when given a $card card", ({ card, value }) => { | ||
| expect(getCardValue(card)).toEqual(value); | ||
| }); | ||
| // -----remaining face cards----- | ||
| test(`Should return 10 when given a jack card`, () => { | ||
| expect(getCardValue("J♥")).toEqual(10); | ||
| }); | ||
| test(`Should return 10 when given a king card`, () => { | ||
| expect(getCardValue("K♣")).toEqual(10); | ||
| }); | ||
| test(`Should return 10 when given a queen card`, () => { | ||
| expect(getCardValue("Q♥")).toEqual(10); | ||
| }); | ||
| // -----invalid cards----- | ||
| test("Should throw an error for card without suit", () => { | ||
| expect(() => getCardValue("A")).toThrow("Missing a specific suit symbol"); | ||
| }); | ||
|
|
||
| test("Should throw an error for card with invalid number", () => { | ||
| expect(() => getCardValue("0♥")).toThrow( | ||
| "Missing a specific number and specific suit symbol" | ||
| ); | ||
| }); | ||
|
|
||
| test("Should throw an error for empty input", () => { | ||
| expect(() => getCardValue("")).toThrow( | ||
| "Missing a specific number and specific suit symbol" | ||
| ); | ||
| }); | ||
|
|
||
| test("Should throw an error for an invalid input", () => { | ||
| expect(() => getCardValue("J♥♥")).toThrow( | ||
| "Missing a specific number and specific suit symbol" | ||
| ); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think happens when a function is declared twice, once at the top and again right inside itself without the inner one ever being called?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to have a read about it and it seems that the function would be redefined inside of itself resulting in the inner function being called instead, but only if it needed to be a recursive one. In this situation, it would look confusing, but would still do the intended job.
However, I think there might have been a misunderstanding with the diff. I only changed the function declaration/export style slightly, so the original declaration in red was replaced by the implementation in green; the function isn't declared twice. I wondered about this because a similar comment was added to another task (implement-isProperFraction) on a function that was also in the deleted code. Could you clarify if I'm missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are spot on! That was entirely on my end due to viewing the diff in Unified View, which made the replaced code look like a duplicate declaration. Your implementation looks completely correct, no double declaration at all. Thanks for clearing that up, and great eye on catching that diff quirk!