West Midlands | 25-ITP-May | Mustaf Asani | Module-Structuring-and -Testing-Data | Sprint 3 #679
Conversation
asaniDev
added
the
Needs Review
LonMcGregor
added
Review in progress
| @@ -9,6 +9,11 @@ | |||
|
|
|||
| function isProperFraction(numerator, denominator) { | |||
| if (numerator < denominator) return true; | |||
There was a problem hiding this comment.
What happens if I give it a negative improper fraction, like -20/2?
| return 11; | ||
| } else if (rank === "K" || rank === "Q" || rank === "J" || rank === "10") { | ||
| return 10; | ||
| }else if (rank >= "2" && rank <= "9") { |
There was a problem hiding this comment.
If I give it a strange value like "222♥" what would you expect the output to be? What actually happens?
| @@ -1,6 +1,12 @@ | |||
| function isProperFraction(numerator, denominator) { | |||
| if (numerator < denominator) return true; | |||
| if (Math.abs(numerator) < Math.abs(denominator)) return true; | |||
There was a problem hiding this comment.
This is a more correct solution - you may want to use it in your earlier task
There was a problem hiding this comment.
Don't commit package files unless you are making a deliberate necessary change to the node setup - can you remove it from here, and the other places it is present in this commit?
| return "3rd"; | ||
| case 4: | ||
| return "4th"; | ||
| case 5: |
There was a problem hiding this comment.
There is a lot of duplication here - can you think of a way to make this code a bit cleaner?
(Hint: You did a better job in the next part of the if branch)
| } | ||
| } else if(num > 10 && num < 21) { | ||
| return num + "th"; | ||
| } else if (num > 20) { |
There was a problem hiding this comment.
What about 111? We would usually state that as "111th".
| // Case 10: Identify the ordinal number for 10 | ||
| test("should return '10th' for 10", () => { | ||
| expect(getOrdinalNumber(10)).toEqual("10th"); | ||
| }); |
There was a problem hiding this comment.
You should consider testing all your branches - what about the larger numbers?
LonMcGregor
added
Reviewed
Learners, PR Template
Self checklist
Changelist
sprint 3 tasks completed
Questions
Ask any questions you have for your reviewer.