London | 26-ITP-Jan | Divine Mankrado | Sprint 3 | Implement and Rewrite Tests#1196
London | 26-ITP-Jan | Divine Mankrado | Sprint 3 | Implement and Rewrite Tests#1196divinmank wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
divinmank
added
the
Needs Review
favourO
added
🦔 Size Tiny
favourO
left a comment
favourO
left a comment
There was a problem hiding this comment.
Nice job so far,
Please check the comments
|
|
||
| // Example: 1/2 is a proper fraction | ||
| assertEquals(isProperFraction(1, 2), true); | ||
|
|
There was a problem hiding this comment.
Great job, your current implementation treats every negative fraction as false, and your Jest tests are written to match that. But what is the actual rule for a “proper fraction”?
It might be worth checking whether your implementation is based on the maths definition, or whether the tests and implementation have just been made consistent with each other.
There was a problem hiding this comment.
Thanks for pointing this out! I just checked the meaning of a proper fraction and realized that negative fractions can still be proper if the absolute value of the numerator is less than the absolute value of the denominator.
I’ve updated my implementation to use Math.abs() and adjusted the tests to include negative proper fractions as well.
| } catch (e) { } | ||
|
|
||
| // Test invalid rank | ||
| try { |
There was a problem hiding this comment.
It’s good that you’re thinking about invalid inputs as well as valid ones. One small improvement to think about: right now these checks only confirm that some error was thrown. Would it make the test more helpful if it also checked that the error explains the problem clearly?
There was a problem hiding this comment.
Thanks for the suggestion! It makes sense. I’ve just updated the tests so they also check the error message to ensure the error clearly explains the problem with the input.
favourO
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
Finished coursework on sprint 3 implement-rewrite-and-tests.