Manchester| 25-ITP-September | Mahtem T. Mengstu | Sprint 3 | Coursework/sprint 3 practice tdd#874
Manchester| 25-ITP-September | Mahtem T. Mengstu | Sprint 3 | Coursework/sprint 3 practice tdd#874Mahtem wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
Mahtem
added
Needs Review
Sprint-3/2-practice-tdd/count.js
Outdated
| if (typeof stringOfCharacters!= "string") { | ||
| throw new Error("Input should be a string"); | ||
| } | ||
| if (typeof findCharacter !== "string" || findCharacter.length != 1){ | ||
| throw new Error ("Input must be a single character"); | ||
| } | ||
|
|
||
| let count = 0; | ||
| for(let char of stringOfCharacters) { | ||
| if (char === findCharacter) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; | ||
|
|
||
| } |
There was a problem hiding this comment.
Can you improve the indentation?
There was a problem hiding this comment.
Can you improve the indentation?
Thank you, I have improved the indentation.
There was a problem hiding this comment.
Test is quite comprehensive.
Can your function pass all the tests?
There was a problem hiding this comment.
Test is quite comprehensive.
Can your function pass all the tests?
I have added, some more tests and the function passed all tests.
| test("should return correct ordinals for 1, 2, 3, 4, 21, 22, 23, 24", () => { | ||
| expect(getOrdinalNumber(1)).toEqual("1st"); | ||
| expect(getOrdinalNumber(2)).toEqual("2nd"); | ||
| expect(getOrdinalNumber(3)).toEqual("3rd"); | ||
| expect(getOrdinalNumber(4)).toEqual("4th"); | ||
| expect(getOrdinalNumber(21)).toEqual("21st"); | ||
| expect(getOrdinalNumber(22)).toEqual("22nd"); | ||
| expect(getOrdinalNumber(23)).toEqual("23rd"); | ||
| expect(getOrdinalNumber(24)).toEqual("24th"); | ||
| }); | ||
|
|
||
| // Case 2: Exceptions 11, 12, 13 | ||
| test("should handle exceptions 11, 12, 13 correctly", () => { | ||
| expect(getOrdinalNumber(11)).toEqual("11th"); | ||
| expect(getOrdinalNumber(12)).toEqual("12th"); | ||
| expect(getOrdinalNumber(13)).toEqual("13th"); | ||
| expect(getOrdinalNumber(111)).toEqual("111th"); | ||
| }); |
There was a problem hiding this comment.
Could consider grouping test values by their characteristics. For examples,
- All numbers that end in 1 (but not in 11).
- Repeat the above for 2 and 3.
- All numbers that end in 11, 12, or 13.
- All numbers that end in 0, 4-9.
(In addition to invalid cases).
This approach could better cover all possible cases and give more specific message when a test fails.
There was a problem hiding this comment.
Could consider grouping test values by their characteristics. For examples,
* All numbers that end in 1 (but not in 11). * Repeat the above for 2 and 3. * All numbers that end in 11, 12, or 13. * All numbers that end in 0, 4-9. (In addition to invalid cases).This approach could better cover all possible cases and give more specific message when a test fails.
Thank you, I have tried to group them with exceptions.
cjyuan
added
Reviewed
Mahtem
added
the
Needs Review
Sprint-3/2-practice-tdd/count.js
Outdated
| if (typeof stringOfCharacters!= "string") { | ||
| throw new Error("Input should be a string"); | ||
| } | ||
| if (typeof findCharacter !== "string" || findCharacter.length != 1){ | ||
| throw new Error ("Input must be a single character"); | ||
| } | ||
|
|
||
| let count = 0; | ||
|
|
||
| for(let char of stringOfCharacters) { | ||
| if (char === findCharacter) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; | ||
| } |
There was a problem hiding this comment.
Code is still not properly indented.
You can use VSCode's "Format document" feature (after installing the "prettier" extension) to auto-format JS code.
There was a problem hiding this comment.
@cjyuan, I have used prettier to format the document. Thank you.
cjyuan
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
Functions implemented and tested using possible cases.
Questions
Questions will be asked in Slack.