West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 3 | Sprint 3 TDD Coursework - #1604
West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 3 | Sprint 3 TDD Coursework#1604maryam-devio wants to merge 5 commits into
Conversation
maryam-devio
added
🐂 Size Medium
hackertainment
added
Review in progress
There was a problem hiding this comment.
The given example of aaaaa is a special case. There should be more general cases for multiple occurences.
Also, there is one more boundary case for no occurrences. Can you think of it?
hackertainment
left a comment
hackertainment
left a comment
There was a problem hiding this comment.
You generally got the right direction and just need to clear the logic a bit. Good work and keep it up :-)
| function getOrdinalNumber(num) { | ||
| return "1st"; | ||
| let getStr = num.toString(); | ||
| let lastTwoDigit = getStr.slice(-2); |
There was a problem hiding this comment.
while this line works even when the string only contains a single digit, it would be hard to maintain in a more complex project.
There was a problem hiding this comment.
There is no test case for the lastTwoDigit part of your function. Please add them back. Thanks.
There was a problem hiding this comment.
Please tidy-up the indentation so that it would be easier to read. It is also a good and important practice as well.
| } | ||
| return result; | ||
| } | ||
| else if (count == 0){ |
| // The goal is to re-implement that function, not to use it. | ||
| return "hellohellohello"; | ||
| let result=""; | ||
| if (count >=1){ |
There was a problem hiding this comment.
How about empty string (with the same set of counts 3, 1, 0, -1)?
hackertainment
added
Reviewed
|
Hello, thank you for your valuable feedback. I learned from all the comments you made. I've updated my code and pushed the changes. Thanks again! |
2 participants
Learners, PR Template
Self checklist
Changelist
I have attempt all the TDD coursework according to the requirments and test all the changes.