Sheffield | ITP-Jan-26 | Hayriye Saricicek | Sprint 3 | practice tdd#1237
Open
mshayriyesaricicek wants to merge 17 commits intoCodeYourFuture:mainfrom
Open
Sheffield | ITP-Jan-26 | Hayriye Saricicek | Sprint 3 | practice tdd#1237mshayriyesaricicek wants to merge 17 commits intoCodeYourFuture:mainfrom
mshayriyesaricicek wants to merge 17 commits intoCodeYourFuture:mainfrom
Conversation
mshayriyesaricicek
added
Needs Review
cjyuan
reviewed
Mar 13, 2026
cjyuan
added
Reviewed
mshayriyesaricicek
added
Needs Review
cjyuan
reviewed
Mar 15, 2026
Comment on lines
+27
to
+31
| expect(countChar("aaaaa", 'b")).toEqual(5); | ||
| expect(countChar("blind", 'a)).toEqual(0); | ||
| expect(countChar("blood", 'o")).toEqual(2); | ||
| expect(countChar("bbbrf", 'b")).toEqual(3); | ||
| expect(countChar("ooooa", 'o")).toEqual(4); |
Author
There was a problem hiding this comment.
I noticed I had syntax errors with the quotes and when I changed it then saved the file Prettier changed the indentation.
Contributor
There was a problem hiding this comment.
Note: In VSCode, when a JS file ha syntax errors, its name is usually shown in red color in the Explorer view.
cjyuan
added
Reviewed
mshayriyesaricicek
added
Needs Review
cjyuan
reviewed
Mar 15, 2026
Contributor
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good.
I have a few more suggestions.
| function getOrdinalNumber(num) { | ||
| return "1st"; | ||
| // Check if input is a number | ||
| if (!Number.isInteger(num) || !Number.isFinite(num) || num <= 0) { |
Contributor
There was a problem hiding this comment.
-
Number.isFinite()is also redundant -
Good thinking about rejecting negative numbers. In some context such as in math, the ordinal number of
0is0th.
| // For numbers that don't end in 1 (not11), 2 and 3 | ||
| // the function should return a string by appending "th" to the number. | ||
|
|
||
| test("should append 'th' if number is 11, 12 or 13 or does not end in 1, 2 or 3",() => { |
Contributor
There was a problem hiding this comment.
This group should also include numbers ending with 11, 12, or 13 (not just that three numbers).
A direct listing of all the trailing digits in the description might be easier to read:
should append 'th' for numbers ending with 0, 4, 5, 6, 7, 8, 9, 11, 12, or 13
cjyuan
added
Reviewed
mshayriyesaricicek
added
Needs Review
Contributor
|
Changes look good. Well done. |
cjyuan
added
Complete
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Self checklist
Changelist
This PR is to practice writing tests before writing functions