Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
7d498f5 to
0c75ac6
Compare
hackertainment
left a comment
There was a problem hiding this comment.
Thank you for your coursework. Just an additional note: a decimal number itself is a fraction, but decimal fraction may not be well defined without simplified into integer fraction in mathematics. Therefore, I will let you pass this time as this is a programming exercise but not a mathematics course (and you did return the results correctly if I calculate their integer form on my own).
| return "Obtuse angle"; | ||
| } else if (angle === 180) { | ||
| return "Straight angle"; | ||
| } else if (angle > 180 && angle <= 360) { |
There was a problem hiding this comment.
May possibly be just a typo, but read line 10 of the question carefully and please fix your code as well. Thank you.
There was a problem hiding this comment.
Thanks for the review and feedback.
= sign is removed from reflex angle if condition statement.
| const Reflex2 = getAngleType(360); | ||
| assert.equal(Reflex2, "Reflex angle"); |
There was a problem hiding this comment.
This looks not quite correct to me, and please fix it as well. Thank you.
There was a problem hiding this comment.
Hmm... you would also need to change the boundary case of invalid angle as well.
| test(`should return "Reflex angle" when (180 > angle <= 360)`, () => { | ||
| expect(getAngleType(181)).toEqual("Reflex angle"); | ||
| expect(getAngleType(299)).toEqual("Reflex angle"); | ||
| expect(getAngleType(360)).toEqual("Reflex angle"); |
There was a problem hiding this comment.
This line looks not quite correct to me, and please fix it as well. Thank you.
There was a problem hiding this comment.
Hmm... you would also need to change the test description and the boundary case of invalid angle as well.
| test("Negative numerator proper fraction", () => { | ||
| assert.equal(isProperFraction(-9, 1), true); | ||
| }); |
There was a problem hiding this comment.
this one should return false, please fix it, thank you.
| test("Both negative proper fraction", () => { | ||
| assert.equal(isProperFraction(-7, -3), true); | ||
| }); |
There was a problem hiding this comment.
this one should return false, please fix it, thank you.
There was a problem hiding this comment.
Thank you. For both negative numerator and denominator, you have included (-4, -4) and (-7, -3), but how about (-3, -7) where the "result" is a positive number?
There was a problem hiding this comment.
Yes the result would be a positive 3/7 which is a proper fraction.
| test(`should return true for negative proper fraction`, () => { | ||
| expect(isProperFraction(-7, -3)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
this one should return false, please fix it, thank you.
There was a problem hiding this comment.
Thank you. For both negative numerator and denominator, you have included (-4, -4) and (-7, -3), but how about (-3, -7) where the "result" is a positive number?
There was a problem hiding this comment.
Yes the result would be a positive 3/7 which is a proper fraction.
| test(`should return true for negative numerator proper fraction`, () => { | ||
| expect(isProperFraction(-9, 1)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
this one should return false, please fix it, thank you.
There was a problem hiding this comment.
Done.
The reason I included negative fraction is the requirement didn't say no negative numbers. Secondly I checked online and I read that negative numbers can be proper fraction.
There was a problem hiding this comment.
Yes, negative numbers can be proper fraction if (without the negative sign) the numerator is smaller than the denominator. As a result, both (-1, 9) and (1, -9) should return true but (-9, 1) and (9, -1) should return false. The issue here is inconsistency. Either consistently treating end result between -1 and 0 as true if negative numbers can be proper fraction, or consistently treating all negative "result" as false if negative numbers cannot be proper fraction.
p.s.: for this question, you have to handle negative numbers even the question didn't mentioned it.
There was a problem hiding this comment.
I don't think this program is completely correct. Consider card = "AA", would the result be the same as expected?
There was a problem hiding this comment.
I added suits and validSuits variables to fix the bug.
Many thanks
|
For the following two comments, has the test case |
Learners, PR Template
Self checklist
Task code
CYF-1059
Changelist
Functions implemented. Tests written with node and jest.