-
-
Notifications
You must be signed in to change notification settings - Fork 387
London | 25-ITP-Sep | Adnaan Abo | Sprint 3 | coursework/sprint-3-practice-tdd #718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
889f34e
c40ecef
f097ae0
09a628b
a87356b
8b8e22f
b31f116
daf6efa
c7bc598
4183f3b
c3eae64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,12 @@ | ||
| function countChar(stringOfCharacters, findCharacter) { | ||
| return 5 | ||
| function countChar(str, char) { | ||
| if (!char || char.length !== 1) return 0; // ensure char is a single character | ||
| let count = 0; | ||
| for (let i = 0; i < str.length; i++) { | ||
| if (str[i] === char) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| module.exports = countChar; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,22 @@ | ||
| function getOrdinalNumber(num) { | ||
| return "1st"; | ||
|
|
||
| if (typeof num !== "number" || !Number.isInteger(num) || num <= 0) { | ||
| return "Invalid input"; | ||
| } | ||
| if (num % 100 >= 11 && num % 100 <= 13) { | ||
| return num + "th"; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation is a bit off.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation is now fixed |
||
| switch (num % 10) { | ||
| case 1: | ||
| return num + "st"; | ||
| case 2: | ||
| return num + "nd"; | ||
| case 3: | ||
| return num + "rd"; | ||
| default: | ||
| return num + "th"; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| module.exports = getOrdinalNumber; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,3 +11,67 @@ const getOrdinalNumber = require("./get-ordinal-number"); | |
| test("should return '1st' for 1", () => { | ||
| expect(getOrdinalNumber(1)).toEqual("1st"); | ||
| }); | ||
|
|
||
| // Case 2: Identify the ordinal number for 2 | ||
| // When the number is 2, | ||
| // Then the function should return "2nd" | ||
|
|
||
| test("should return '2nd' for 2", () => { | ||
| expect(getOrdinalNumber(2)).toEqual("2nd"); | ||
| }); | ||
|
|
||
| // Case 3: Identify the ordinal number for 3 | ||
| // When the number is 3, | ||
| // Then the function should return "3rd" | ||
|
|
||
| test("should return '3rd' for 3", () => { | ||
| expect(getOrdinalNumber(3)).toEqual("3rd"); | ||
| }); | ||
|
|
||
| // Case 4: Identify the ordinal number for 4 | ||
| // When the number is 4, | ||
| // Then the function should return "4th" | ||
|
|
||
| test("should return '4th' for 4", () => { | ||
| expect(getOrdinalNumber(4)).toEqual("4th"); | ||
| }); | ||
|
|
||
| // Case 5: Identify the ordinal number for 11 | ||
| // When the number is 11, | ||
| // Then the function should return "11th" | ||
|
|
||
| test("should return '11th' for 11", () => { | ||
| expect(getOrdinalNumber(11)).toEqual("11th"); | ||
| }); | ||
|
|
||
| // Case 6: Identify the ordinal number for 21 | ||
| // When the number is 21, | ||
| // Then the function should return "21st" | ||
|
|
||
| test("should return '21st' for 21", () => { | ||
| expect(getOrdinalNumber(21)).toEqual("21st"); | ||
| }); | ||
|
|
||
| // Case 7: Identify the ordinal number for 102 | ||
| // When the number is 102, | ||
| // Then the function should return "102nd" | ||
|
|
||
| test("should return '102nd' for 102", () => { | ||
| expect(getOrdinalNumber(102)).toEqual("102nd"); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To ensure thorough testing, we need broad scenario coverage. Listing individual values, however, can quickly lead to an unmanageable number of test cases. For example, we can prepare a test for numbers 2, 22, 132, etc. as
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the feedback I have grouped all the different teste-cases to categories. some of the benefits of this approach are:
|
||
|
|
||
| // Case 8: Identify the ordinal number for 111 | ||
| // When the number is 111, | ||
| // Then the function should return "111th" | ||
|
|
||
| test("should return '111th' for 111", () => { | ||
| expect(getOrdinalNumber(111)).toEqual("111th"); | ||
| }); | ||
|
|
||
| // case 9: identify the ordinal number for 211 | ||
| // when the number is 211 | ||
| // then the function should return "211th" | ||
|
|
||
| test("should return '33rd' for 33", () => { | ||
| expect(getOrdinalNumber(33)).toEqual("33rd"); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,31 @@ | ||
| function repeat() { | ||
| return "hellohellohello"; | ||
| if (arguments.length !== 2) { | ||
| throw new Error("Function requires exactly two arguments: str and count."); | ||
| } | ||
|
|
||
| const [str, count] = arguments; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just declare the parameters as function repeat(str, count) {
...?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we can declare the parameters in the function call and we will not need to declare const [str, count] = arguments;. |
||
| if (typeof str !== "string") { | ||
| throw new Error("First argument must be a string."); | ||
| } | ||
|
|
||
| if (typeof count !== "number" || !Number.isInteger(count) || count < 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the case in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two other ways that this condition can be expressed concisely. concise (but potentially less readable): and the other concise but more readable option: |
||
| throw new Error("Second argument must be a non-negative integer."); | ||
| } | ||
|
|
||
| if (count === 0) { | ||
| return ""; | ||
| } | ||
|
|
||
| if (count === 1) { | ||
| return str; | ||
| } | ||
|
|
||
| let result = ""; | ||
| for (let i = 0; i < count; i++) { | ||
| result += str; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| module.exports = repeat; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you think of a more concise way to express the condition of this if statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
Thank you for the feedback.
Yes the If statement can be made more concise. Here’s a more compact version:
if (!Number.isInteger(num) || num <= 0) {