-
-
Notifications
You must be signed in to change notification settings - Fork 260
London | 25-ITP-Sep | Shaghayegh Shirinfar | Sprint 3 | Practice-tdd #818
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 5 commits
3289b60
47c3136
87552c8
4386c93
a8355cf
ce61aaf
7795cdb
3fe83f1
4b078fa
9861412
29a4b36
1907463
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,11 @@ | ||
| function countChar(stringOfCharacters, findCharacter) { | ||
| return 5 | ||
| let count = 0; | ||
| for (let i = 0; i < stringOfCharacters.length; i++) { | ||
| if (stringOfCharacters[i] === findCharacter) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; | ||
| } | ||
|
|
||
| module.exports = countChar; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,50 @@ test("should count multiple occurrences of a character", () => { | |
| expect(count).toEqual(5); | ||
| }); | ||
|
|
||
| test("should count multiple occurrences of a character", () => { | ||
|
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. Okay, this is a good improvement on the previous iteration. Can you think of corner cases? What if length of stringOfCharacters is shorter than length of findCharacter? What either of them is an empty string or some other data type? |
||
| const str = "bbb"; | ||
| const char = "b"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(3); | ||
| }); | ||
|
|
||
| test("should count mutiple of character 'coco can have coconut'", () => { | ||
| const str = "coco can have coconut"; | ||
| const char = "c"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(5); | ||
| }); | ||
|
|
||
| test("should count multiple occurrences of a character 'a' in 'I have an apple'", () => { | ||
| const str = "I have an apple"; | ||
| const char = "a"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(3); | ||
| }); | ||
|
|
||
| // Scenario: No Occurrences | ||
| // Given the input string str, | ||
| // And a character char that does not exist within the case-sensitive str, | ||
| // When the function is called with these inputs, | ||
| // Then it should return 0, indicating that no occurrences of the char were found in the case-sensitive str. | ||
|
|
||
| test("should return 0 when the character does not exist in the string", () => { | ||
|
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. This test suite is very slim, can you think of more test cases? You should try to cover as many scenarios as possible to test whether your function behaves right if something expected or unexpected passed to your function. |
||
| const str = "hello"; | ||
| const char = "z"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(0); | ||
| }); | ||
|
|
||
| test("should return 0 when the character does not exist in the string", () => { | ||
|
||
| const str = "hello"; | ||
| const char = "z"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(0); | ||
| }); | ||
|
|
||
| test("should return 0 when the 'c' does not exist in string 'I dont have an apple'", () => { | ||
| const str = "I dont have an apple"; | ||
| const char = "c"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(0); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,14 @@ | ||
| function getOrdinalNumber(num) { | ||
| return "1st"; | ||
| function getOrdinalNumber(number) { | ||
|
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. Unfortunately, this function is not passing the acceptance criteria. What if I pass, for example, 23 to it? |
||
| if (number % 100 >= 11 && number % 100 <= 13) { | ||
| return number + "th"; | ||
| } else { | ||
| const lastdigit = number.toString().slice(-1); | ||
|
|
||
| if (lastdigit == 1) return number + "st"; | ||
| if (lastdigit == 2) return number + "nd"; | ||
| if (lastdigit == 3) return number + "rd"; | ||
| return number + "th"; | ||
| } | ||
| } | ||
|
|
||
| module.exports = getOrdinalNumber; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| // get-ordinal-number.test.js | ||
| const getOrdinalNumber = require("./get-ordinal-number"); | ||
| // In this week's prep, we started implementing getOrdinalNumber | ||
|
|
||
|
|
@@ -7,7 +8,62 @@ const getOrdinalNumber = require("./get-ordinal-number"); | |
| // Case 1: Identify the ordinal number for 1 | ||
| // When the number is 1, | ||
| // Then the function should return "1st" | ||
|
|
||
| test("should return '1st' for 1", () => { | ||
| expect(getOrdinalNumber(1)).toEqual("1st"); | ||
| }); | ||
|
|
||
| // Case 2: Identify the ordinal number for 2 | ||
|
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. I think it's a good illustration why you need to write more versatile unit tests. Before fixing your function, add more unit tests (for different numbers) and see if they pass. You will see that they do not and this is a good sign that your function implementation is incorrect. |
||
| // When the number is 2, | ||
| // Then the function should return "2nd" | ||
|
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. Same as the comment for |
||
| 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 9: Identify the ordinal number for 9 | ||
| // When the number is 9, | ||
| // Then the function should return "4th" | ||
| test("should return '9th' for 9", () => { | ||
| expect(getOrdinalNumber(9)).toEqual("9th"); | ||
| }); | ||
|
|
||
| // Case 10: Identify the ordinal number for 10 | ||
| // When the number is 10, | ||
| // Then the function should return "4th" | ||
| test("should return '10th' for 10", () => { | ||
| expect(getOrdinalNumber(10)).toEqual("10th"); | ||
| }); | ||
|
|
||
| // Case 11: Identify the ordinal number for 11 | ||
| // When the number is 11, | ||
| // Then the function should return "4th" | ||
| test("should return '11th' for 11", () => { | ||
| expect(getOrdinalNumber(11)).toEqual("11th"); | ||
| }); | ||
|
|
||
| // Case 20: Identify the ordinal number for 20 | ||
| // When the number is 20, | ||
| // Then the function should return "20rd" | ||
| test("should return '20th' for 20", () => { | ||
| expect(getOrdinalNumber(20)).toEqual("20th"); | ||
| }); | ||
|
|
||
| // Case 23: Identify the ordinal number for 23 | ||
| // When the number is 23, | ||
| // Then the function should return "33rd" | ||
| test("should return '23rd' for 23", () => { | ||
| expect(getOrdinalNumber(23)).toEqual("23rd"); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "name": "2-practice-tdd", | ||
| "version": "1.0.0", | ||
| "description": "In this section you'll practice this key skill of building up your program test first.", | ||
| "main": "count.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| }, | ||
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC", | ||
| "type": "commonjs" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,30 @@ | ||
| function repeat() { | ||
| return "hellohellohello"; | ||
| function repeat(str, count) { | ||
| if (str === undefined) { | ||
| throw new Error("String must be defined"); | ||
| } | ||
| if (str === null || typeof str !== "string") { | ||
| throw new Error("String must be a valid string"); | ||
| } | ||
|
|
||
| if (count === undefined || typeof count !== "number") { | ||
| throw new Error("Count must be a number"); | ||
| } | ||
|
|
||
| if (count < 0 || !Number.isInteger(count)) { | ||
| throw new Error("Count must be a non-negative integer"); | ||
| } | ||
|
|
||
| if (str === "" || count === 0) { | ||
| return ""; | ||
| } | ||
|
Comment on lines
+2
to
+19
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. This is okay but it could be simplified by combining some of these if conditions and/or writing more general if condition that would catch most of the cases. You don't need to do anything, it's just something to think about and perhaps to implement on your spare time. |
||
|
|
||
| // 5. Repeat the string count times | ||
|
||
| let result = ""; | ||
| for (let i = 0; i < count; i++) { | ||
| result += str; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| module.exports = repeat; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // Implement a function repeat | ||
| const repeat = require("./repeat"); | ||
| // Given a target string str and a positive integer count, | ||
| // When the repeat function is called with these inputs, | ||
|
|
@@ -21,12 +20,69 @@ test("should repeat the string count times", () => { | |
| // When the repeat function is called with these inputs, | ||
| // Then it should return the original str without repetition, ensuring that a count of 1 results in no repetition. | ||
|
|
||
| test("should return the original string when count is 1", () => { | ||
|
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. These unit tests are correct but they are a bit limited. What if I pass undefined as str and count 2? What if I pass empty string? You should try to guard your function against these scenarios and unit tests help to think of different scenarios. |
||
| const str = "hi"; | ||
| const count = 1; | ||
| const repeatedStr = repeat(str, count); | ||
| expect(repeatedStr).toEqual("hi"); | ||
| }); | ||
|
|
||
| // case: Handle Count of 0: | ||
| // Given a target string str and a count equal to 0, | ||
| // When the repeat function is called with these inputs, | ||
| // Then it should return an empty string, ensuring that a count of 0 results in an empty output. | ||
|
|
||
| test("should return an empty string when count is 0", () => { | ||
| const str = "hi"; | ||
| const count = 0; | ||
| const repeatedStr = repeat(str, count); | ||
| expect(repeatedStr).toEqual(""); | ||
| }); | ||
|
|
||
| // case: Negative Count: | ||
| // Given a target string str and a negative integer count, | ||
| // When the repeat function is called with these inputs, | ||
| // Then it should throw an error or return an appropriate error message, as negative counts are not valid. | ||
|
|
||
| test("should throw an error when count is negative", () => { | ||
| const str = "hi"; | ||
| const count = -2; | ||
| expect(() => repeat(str, count)).toThrow( | ||
| "Count must be a non-negative integer" | ||
| ); | ||
| }); | ||
|
|
||
| // case: Undefined String | ||
| // Given that str is undefined and count is a positive integer, | ||
| // When the repeat function is called with these inputs, | ||
| // Then it should throw an error or return an appropriate message, | ||
| // since repeating an undefined value does not make sense. | ||
|
|
||
| test("should throw an error when string is undefined", () => { | ||
| const str = undefined; | ||
| const count = 2; | ||
| expect(() => repeat(str, count)).toThrow("String must be defined"); | ||
| }); | ||
|
|
||
| // case: Empty String | ||
| // Given an empty string str and a positive integer count, | ||
| // When the repeat function is called, | ||
| // Then it should return an empty string, since there is nothing to repeat. | ||
|
|
||
| test("should return an empty string when input string is empty", () => { | ||
| const str = ""; | ||
| const count = 3; | ||
| const repeatedStr = repeat(str, count); | ||
| expect(repeatedStr).toEqual(""); | ||
| }); | ||
|
|
||
| // case: Null String | ||
| // Given that str is null and count is a positive integer, | ||
| // When the repeat function is called, | ||
| // Then it should throw an error, because null is not a valid string input. | ||
|
|
||
| test("should throw an error when string is null", () => { | ||
| const str = null; | ||
| const count = 2; | ||
| expect(() => repeat(str, count)).toThrow("String must be a valid string"); | ||
| }); | ||
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.
Nice use of for loop!