London | 26-ITP-May | Edina Kurdi | Sprint 3 | Implement and rewrite tests#1468
London | 26-ITP-May | Edina Kurdi | Sprint 3 | Implement and rewrite tests#1468edinakurdi wants to merge 8 commits into
Conversation
| const straight = getAngleType(180); | ||
| assertEquals(straight, "Straight angle"); | ||
|
|
||
| const acute_upper_boundary = getAngleType(89); | ||
| assertEquals(acute_upper_boundary, "Acute angle"); | ||
|
|
||
| const acute_lower_boundary = getAngleType(1); | ||
| assertEquals(acute_lower_boundary, "Acute angle"); | ||
|
|
||
| const invalid_negative = getAngleType(-1); | ||
| assertEquals(invalid_negative, "Invalid angle"); | ||
|
|
||
| const invalid_zero = getAngleType(0); | ||
| assertEquals(invalid_zero, "Invalid angle"); | ||
|
|
||
| const invalid_full = getAngleType(360); | ||
| assertEquals(invalid_full, "Invalid angle"); | ||
|
|
||
| const invalid_big = getAngleType(361); | ||
| assertEquals(invalid_big, "Invalid angle"); |
There was a problem hiding this comment.
NNice touch using the variable names to explain the test case. This makes it easier to understand
| const acute_decimal = getAngleType(89.9); | ||
| assertEquals(acute_decimal, "Acute angle"); | ||
|
|
||
| const obtuse_decimal = getAngleType(179.9); | ||
| assertEquals(obtuse_decimal, "Obtuse angle"); | ||
|
|
||
| const reflex_decimal = getAngleType(180.1); | ||
| assertEquals(reflex_decimal, "Reflex angle"); |
| expect(getAngleType(90)).toEqual("Right angle"); | ||
| }); | ||
|
|
||
| // Case 3: Obtuse angles | ||
| test('should return "Obtuse angle" when (90 < angle < 180', () => { | ||
| // Test various obtuse angles, including boundary cases | ||
| expect(getAngleType(91)).toEqual("Obtuse angle"); | ||
| expect(getAngleType(100)).toEqual("Obtuse angle"); | ||
| expect(getAngleType(179)).toEqual("Obtuse angle"); | ||
| }); | ||
|
|
||
| // Case 4: Straight angle | ||
| test('should return "Straight angle" when (angle = 180)', () => { | ||
| // Test straight angle | ||
| expect(getAngleType(180)).toEqual("Straight angle"); | ||
| }); | ||
|
|
||
| // Case 5: Reflex angles | ||
| test('should return "Reflex angle" when (180 < angle < 360', () => { | ||
| // Test various reflex angles, including boundary cases | ||
| expect(getAngleType(181)).toEqual("Reflex angle"); | ||
| expect(getAngleType(300)).toEqual("Reflex angle"); | ||
| expect(getAngleType(359)).toEqual("Reflex angle"); | ||
| }); | ||
|
|
||
| // Case 6: Invalid angles | ||
| test('Should return "Invalid angle" when the input is invalid', () => { | ||
| expect(getAngleType(-1)).toEqual("Invalid angle"); | ||
| expect(getAngleType(360)).toEqual("Invalid angle"); | ||
| expect(getAngleType(0)).toEqual("Invalid angle"); | ||
| expect(getAngleType(361)).toEqual("Invalid angle"); |
There was a problem hiding this comment.
Well done on testing all the boundaries
|
|
||
| function isProperFraction(numerator, denominator) { | ||
| // TODO: Implement this function | ||
| if (denominator === 0 || numerator === 0) return false; |
There was a problem hiding this comment.
Because then we have a 0 as a result, which is not a fraction. Although now that I think about it 0/9 is still a fraction, even if the result isn't... . Amended.
| if (card[0] === "A" && suits.includes(card[1]) && card.length === 2) | ||
| return 11; | ||
| if ( | ||
| (card[0] === "J" || card[0] === "Q" || card[0] === "K") && | ||
| suits.includes(card[1]) && | ||
| card.length === 2 | ||
| ) | ||
| return 10; | ||
| if ( | ||
| ["2", "3", "4", "5", "6", "7", "8", "9"].includes(card[0]) && | ||
| suits.includes(card[1]) && | ||
| card.length === 2 | ||
| ) | ||
| return Number(card[0]); | ||
| if (card.slice(0, 2) == "10" && suits.includes(card[2]) && card.length === 3) | ||
| return 10; | ||
| throw new Error( | ||
| 'Invalid input: Expected rank followed by a suit symbol. For example: "A♠", "2♥", "10♥", "J♣", "Q♦", "K♦"' |
There was a problem hiding this comment.
I find the code hard to read. I like the allowed suits definition. Can you do something similar with the numbers. How can you determine suit and rank of a card. (A different way then access by index might be simpler)
There was a problem hiding this comment.
i refactored the code now it looks simpler and shorter and hopefully more readable
| getCardValue("9♠ "); | ||
|
|
||
| // This line will not be reached if an error is thrown as expected | ||
| console.error("Error was not thrown for invalid card 😢"); | ||
| } catch (e) { | ||
| console.log("Error thrown for invalid card 🎉"); | ||
| } | ||
|
|
||
| try { | ||
| getCardValue("♠9"); | ||
|
|
||
| // This line will not be reached if an error is thrown as expected | ||
| console.error("Error was not thrown for invalid card 😢"); | ||
| } catch (e) { | ||
| console.log("Error thrown for invalid card 🎉"); | ||
| } | ||
|
|
||
| try { | ||
| getCardValue("99♠"); | ||
|
|
||
| // This line will not be reached if an error is thrown as expected | ||
| console.error("Error was not thrown for invalid card 😢"); | ||
| } catch (e) { | ||
| console.log("Error thrown for invalid card 🎉"); | ||
| } | ||
|
|
||
| try { | ||
| getCardValue("9♠♠"); | ||
|
|
||
| // This line will not be reached if an error is thrown as expected | ||
| console.error("Error was not thrown for invalid card 😢"); | ||
| } catch (e) { | ||
| console.log("Error thrown for invalid card 🎉"); | ||
| } | ||
|
|
||
| try { | ||
| getCardValue("9❤️"); |
There was a problem hiding this comment.
Nice work. You found a lot of error cases
Learners, PR Template
Self checklist
Changelist
Implemented and tested angles, proper fraction and cards functions, also tested using jest
Questions
With the card implementation, I tried making it cleaner, but nested if-statements are not the best for clean code. Is that correct? Would there be a more structured way of dealing with this? I think I was on the right track with creating the suits array inside the function, but then it got a bit messy. Are there any techniques that would help me gear towards a better solution here?
Also, when trying to write tests, how many of the same kind is enough? for instance, with the cards, would i need to test for each number value when its a number card, or is it enough to test for 2-3 numbers?