Repository navigation
London | 26-Sep-ITP | Frumentius Tesfay | Sprint 2 | Coursework/sprint 3 implement and rewrite - #1698
Conversation
Frumentius-Rev
added
the
Needs Review
hey-hammad
added
Review in progress
hey-hammad
left a comment
hey-hammad
left a comment
There was a problem hiding this comment.
Good implementation overall. Thanks
|
|
||
| export function isProperFraction(numerator, denominator) { | ||
| // TODO: Implement this function | ||
| return numerator < denominator; |
There was a problem hiding this comment.
what happens if negative values are passed as input?
There was a problem hiding this comment.
Negative values are compared normally. Should we allow them as valid inputs?
There was a problem hiding this comment.
Yes negative number in this case are valid input and should be handled accordingly.
However, the standard definition of a proper fraction focuses on the absolute values of the numerator and denominator. Is there a way to handle absolute values in js?
hey-hammad
added
Reviewed
Frumentius-Rev
added
the
Needs Review
hey-hammad
added
Review in progress
hey-hammad
left a comment
hey-hammad
left a comment
There was a problem hiding this comment.
Implementation is good overall, however test cases can be improved.
- Get Angle Type: the suite should cover boundary values like 0, 89, 90, 179, 180, 181, 359, 360 etc to make the behaviour unambiguous.
- Is Proper Fraction: the suite should cover negative values, zero numerator, zero denominator, and sign combinations.
- Get Card Value: the suite seems incomplete, it should cover all the cases e.g
- it should return when Ace → 11(for all the validSuits)
- it should also cover all the cases when the card is face card or a number card.
- A better way of doing this is to test Ace, face cards, and all number cards in one loop/array, then test invalid strings separately with exact error assertions for bad rank, bad suit, and malformed input. That makes the suite more complete, easier to read, and much better at catching regressions.
|
|
||
| export function isProperFraction(numerator, denominator) { | ||
| // TODO: Implement this function | ||
| return numerator < denominator; |
There was a problem hiding this comment.
Yes negative number in this case are valid input and should be handled accordingly.
However, the standard definition of a proper fraction focuses on the absolute values of the numerator and denominator. Is there a way to handle absolute values in js?
| expect(getCardValue("2♥")).toEqual(2); | ||
| }); | ||
|
|
||
| test("Number card (2-10)", () => { |
There was a problem hiding this comment.
Its better to use test names that describe the expected behavior, not the implementation or category labels.
A better way is to to use the following formula to write test case names:
Behavior + Input + Expected outcome
returns 11 for an ace
hey-hammad
added
Reviewed
Learners, PR Template
Self checklist
Task code
CYF-1059
Changelist
Rewrote tests using Jest with some AI assistance and all tests appear to pass.