London | 26-ITP-Jan | Miriam Jorna | Sprint 3 | Coursework implement and rewrite tests#1269
Conversation
miriamjorna
added
📅 Sprint 3
ykamal
added
Review in progress
| function isProperFraction(numerator, denominator) { | ||
| // TODO: Implement this function | ||
| // denominator cannot be 0 in a proper fraction | ||
| if (denominator === 0) { |
There was a problem hiding this comment.
Excellent check for 0. Whenever it comes to a division, always check for 0. 🎃
|
|
||
| function getCardValue(card) { | ||
| // TODO: Implement this function | ||
| if (typeof card !== "string") { |
There was a problem hiding this comment.
So, what you're trying to ensure is that this argument is provided, is a string and can be used down the line. This is a solid approach to do that 👍
However, you also need to ensure it's long enough.
| @@ -15,7 +15,26 @@ | |||
| // execute the code to ensure all tests pass. | |||
|
|
|||
| function getAngleType(angle) { | |||
There was a problem hiding this comment.
This is good work 👍 You're accounting for most types of angles. But there is a problem here. 0 is also sometimes considered a valid angle. Regardless, if your code assumes that it's valid, it should take that into account, and if it thinks 0 degrees is not a valid angle, it should take that into account as well.
How would you do that?
| test(`should return true when the numerator is zero and the denominator is non-zero`, () => { | ||
| expect(isProperFraction(0, 5)).toEqual(true); | ||
| expect(isProperFraction(0, -5)).toEqual(true); | ||
| } |
There was a problem hiding this comment.
There is a formatting issue here. I think it's worth investing some time in VS Code tooling that can help you find these out, such as Prettier auto-formating on save and code linting.
ykamal
left a comment
ykamal
left a comment
There was a problem hiding this comment.
Overall, very well done 👍
There are a few things here and there. I've left some comments in the corresponding lines.
ykamal
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
Coursework for Sprint 3: Implement and rewrite tests