Sheffield | 25-ITP-Sep | Xiayidan Abuduxukuer | Sprint 3 | Coursework implement and rewrite #881
Conversation
Shayida999
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Can you also revert the change made in the Sprint-1 folder to keep this clean?
| function isProperFraction(numerator,denominator) { | ||
|
|
||
| if (numerator <denominator){ | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| else { | ||
|
|
||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
According to the definition of proper fraction in mathematics:
isProperFraction(-4, 3)should returnfalseisProperFraction(-2, 5)should returntrueisProperFraction(-1, 1)should returnfalseisProperFraction(-2, -3)should returntrue
Can you look up the definition of proper fraction and update your function accordingly?
| test("should return 11 for Ace of Spades",() =>{ | ||
| const aceofSpades = getCardValue("A♠"); | ||
| expect(aceofSpades).toEqual(11); | ||
| }); |
There was a problem hiding this comment.
There is a similar test for Ace on lines 31-37.
There was a problem hiding this comment.
The tests on this script is pretty solid. Have you tried running this script (after you made changes to the implementation of the function in ../implement/3-get-card-value.js) to ensure the function can pass all tests?
Sprint-3/2-practice-tdd/count.js
Outdated
| function countChar(stringOfCharacters, findCharacter){ | ||
|
|
||
| let count =0; | ||
|
|
||
| for (let i = 0;i<stringOfCharacters.length;i++){ | ||
|
|
||
| if (stringOfCharacters[i] ===findCharacter){ | ||
|
|
||
| count =count +1; | ||
| } | ||
| } | ||
|
|
||
| return count; | ||
| } |
There was a problem hiding this comment.
Leaving a space around operators could make the code easier to read. You can consider using the "Format document" feature in VS Code to auto format the code. You may need to install the VSCode "prettier" extension first.
| test("count a's in aaaaa",()=>{ | ||
| expect(countChar("aaaaa","a")).toEqual(5); | ||
| }); | ||
|
|
||
| test("count z in hello",()=>{ | ||
| expect(countChar("hello","z")).toEqual(0); | ||
| }); | ||
|
|
||
| test("count b in abc",()=>{ | ||
| expect(countChar("abc","b")).toEqual(1); | ||
| }); | ||
|
|
||
| test("count a in empty string",()=>{ | ||
| expect(countChar("","a")).toEqual(0); | ||
| }); |
There was a problem hiding this comment.
-
Could consider including the tests on lines 31, 35 in the test category on lines 13-18. (It is better to group related tests into the same test category)
-
Test on line 27 is the same as the test on lines 14-17.
-
In Test-Driven Development, the test description should also describe the expected behavior of the function. Can you update the test description on line 38?
| test("should return '2nd' for 2",() =>{ | ||
| expect(getOrdinalNumber(2)).toEqual("2nd"); | ||
| }); | ||
| test("should return '3rd' for 3",() =>{ | ||
| expect(getOrdinalNumber(3)).toEqual("3rd"); | ||
| }); | ||
| test("should return '11th' for 11",() =>{ | ||
| expect(getOrdinalNumber(11)).toEqual("11th"); | ||
| }); | ||
| test("should return '13th' for 13",() =>{ | ||
| expect(getOrdinalNumber(13)).toEqual("13th"); | ||
| }); |
There was a problem hiding this comment.
To ensure thorough testing, we need broad scenarios that cover all possible cases.
Listing individual values, however, can quickly lead to an unmanageable number of test cases.
Instead of writing tests for individual numbers, consider grouping all possible input values into meaningful categories.
Then, select representative samples from each category to test. This approach improves coverage and makes our tests easier to maintain.
For example, we can prepare a test for numbers 2, 22, 132, etc. as
test("append 'nd' to numbers ending in 2, except those ending in 12", () => {
expect( getOrdinalNumber(2) ).toEqual("2nd");
expect( getOrdinalNumber(22) ).toEqual("22nd");
expect( getOrdinalNumber(132) ).toEqual("132nd");
});
Can you update the tests in this script to make the tests more comprehensive. Ideally, the test categories could cover all valid numbers.
There was a problem hiding this comment.
This script seems to be incomplete.
Since this is a stretch exercise, change is optional.
cjyuan
added
Reviewed
2 participants
Sheffield | 25-ITP-Sep | Xiayidan Abuxuukuer | Sprint 3 | Coursework implement and rewrite
Learners, PR Template
Self checklist
Changelist
Sprint-3/3-stretchdirectory.Questions
none for now thanks.