Manchester | 26-ITP-JAN | Abdu Mussa | Sprint 3 | 2-practice-tdd-complete#1174
Manchester | 26-ITP-JAN | Abdu Mussa | Sprint 3 | 2-practice-tdd-complete#1174Abduhasen wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
Abduhasen
added
📅 Sprint 3
| // Case 1: Numbers ending with 1 (but not 11) | ||
| // When the number ends with 1, except those ending with 11, | ||
| // Then the function should return a string by appending "st" to the number. | ||
| test("should append 'rd' for numbers ending with 3, except those ending with 13", () => { |
There was a problem hiding this comment.
These comments do not match the test being carried out. Do you still need these comments?
The same comments appear at several places in this file.
There was a problem hiding this comment.
I did the first one and i cope and pasted it to save time but I forgot to change the cases i was doing
| test("should repeat the string is empty ", () => { | ||
| let word = "hello"; | ||
| let times = 0; | ||
| const repeatedStr = repeatStr(word, times); | ||
| expect(repeatedStr).toBe(""); | ||
| }); | ||
| // Case: Handle negative count: | ||
| // Given a target string `str` and a negative integer `count`, | ||
| // When the repeatStr function is called with these inputs, | ||
| // Then it should throw an error, as negative counts are not valid. | ||
| test("should repeat the string return negative number not allowed ", () => { |
There was a problem hiding this comment.
These two test descriptions do not quite make sense.
Can you rephrase them, for example, in the format: should <expected_behavior> when <condition> ?
| // When the repeatStr function is called with these inputs, | ||
| // Then it should return the original `str` without repetition. | ||
|
|
||
| test("should repeat the string count times", () => { |
There was a problem hiding this comment.
How about should return the original string when count is 1 (so that the description won't duplicate)?
There was a problem hiding this comment.
It was not correctly described at first now it's fixed.
cjyuan
added
Reviewed
…` ) based on mentor feedback
Abduhasen
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. I just spotted something I missed in my previous review.
| if (times > 0) { | ||
| return word.repeat(times); | ||
| } else if (times < 0) { | ||
| return "Error:negative number not allowed"; |
There was a problem hiding this comment.
I overlooked this in my previous review.
When a function that is expected to return a string is designed to return a string on error,
how should the caller distinguish the return value of these two function calls?
repeatStr("Error:negative number not allowed", 1)repeatStr("Hello World", -1)
They both return the same value.
The requirement is "to throw an error when count is negative". Can you update this implementation and the Jest test accordingly?
There was a problem hiding this comment.
I changed the function and implemented jest test case for negative numbers.
cjyuan
removed
the
Needs Review
Abduhasen
added
the
Needs Review
| // Given a target string `str` and a negative integer `count`, | ||
| // When the repeatStr function is called with these inputs, | ||
| // Then it should throw an error, as negative counts are not valid. | ||
| test("should return a string 'negative number not allowed' when negative number passed", () => { |
There was a problem hiding this comment.
Should also update the test description as well.
There was a problem hiding this comment.
It is fixed now thank you very much for the feed backs
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
writing a function and testing the function.
Questions
N/A