London | 26-ITP-January | Raihan Sharif | Sprint 3 | Coursework/sprint3/2-practice-tdd#1056
London | 26-ITP-January | Raihan Sharif | Sprint 3 | Coursework/sprint3/2-practice-tdd#1056RaihanSharif wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
RaihanSharif
added
the
Needs Review
There was a problem hiding this comment.
-
Could consider testing more samples.
-
Could consider testing these cases:
- A case to show that the match is case sensitive
- A case to show that the function is expected to work also for non-alphabets
| // 11 is a special case | ||
| if (num === 11) { | ||
| return "11th"; | ||
| } |
There was a problem hiding this comment.
11 is not the only special case. Can you look up the rules of ordinal numbers and update also the tests?
| } | ||
|
|
||
| if ((count) => 1) { | ||
| return str.repeat(count); |
There was a problem hiding this comment.
Can you look up the behavior of .repeat(count) when count is 0 or negative?
cjyuan
added
Reviewed
str.repeat() throws Error for values < 0 and return "" for value of -0. Which covers all cases.
|
Made all changes mentioned in review. |
|
Changes look good. Note: If your PR is ready to be re-reviewed, you should also add the "Needs Review" label. |
cjyuan
added
Complete
Will do. Thank you for the review. |
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
3 participants
Learners, PR Template
Self checklist
Changelist
Added necessary implentations and tests