Birmingham | 26-ITP-May | Tobias Amaechina | Sprint 3 | Coursework sprint 3 practice tdd#1501
Conversation
…in 1 excpet 11 and everything else to end in th
Tobias-Amaechina
added
the
Needs Review
| // Case 6: All other numbers | ||
| // When the number does not fall into any of the above categories, | ||
| // Then the function should return a string by appending "th" to the number. | ||
| test("should append 'th' for all other numbers", () => { |
There was a problem hiding this comment.
When a test fails with the message "... all other numbers", it may be unclear what "other numbers" actually refers to.
Could you revise the test description to make it more informative?
There was a problem hiding this comment.
Thanks — that’s helpful feedback. I’ll:
make the test title for the 'other numbers' case more explicit,
| for (let i = 0; i < count; i++) { | ||
| result += str; | ||
| } | ||
| if (count < 0) { | ||
| throw new Error("Count must be a non-negative integer"); | ||
| } |
There was a problem hiding this comment.
Why not perform the check before the for-loop?
There was a problem hiding this comment.
Thanks for this feedback I will move the negative-count validation in repeat-str.js to run before the loop and add tests for negative / non-integer counts
| test("should return 0 when character is not found", () => { | ||
| const str = "hello"; | ||
| const char = "x"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(0); | ||
| }); |
There was a problem hiding this comment.
Could consider testing a few more samples in this script - higher chance to detect bugs in code.
Also, the original specification did not clearly state whether the character match should be case-sensitive.
Most people would probably assume that it is, but to demonstrate our understanding or clarify the assumption we made,
we could add test cases to convey this. For examples,
- 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
There was a problem hiding this comment.
Thanks for the feedback , I will add a few tests to show case sensitivity and non-alphabet behaviour
cjyuan
added
Reviewed
…ing and printing of the functions
Tobias-Amaechina
added
Needs Review
| test("should append 'th' for all other numbers", () => { | ||
| // When the number does not fall into the special 1/2/3 endings (or is one of the teen exceptions 11/12/13), | ||
| // Then the function should return a string by appending "th" to the number. Examples: 4, 14, 20, 100. | ||
| test("should append 'th' for numbers that neither end with 1/2/3 nor are 11/12/13 (e.g. 4, 14, 20, 100)", () => { |
There was a problem hiding this comment.
Why not just states numbers ending with 0, 4, ..., 9?
cjyuan
added
Complete
2 participants
Self checklist
Changelist
I implemented three functions (countChar, getOrdinalNumber, repeatStr) using Test-Driven Development principles. I did wrote comprehensive tests first, then implemented the functions to pass those tests, which aligns with the TDD methodology.