London | 25-ITP-Sep | Payman Issa Baiglu | sprint 3 | 1- implement-and-rewrite #890
Conversation
PaymanIB
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Are these files related to Sprint-3 exercise? If not, can you remove them?
They are making code review less convenient.
If they are local configuration files, you can consider updating .gitignore in the top level folder to exclude them from being tracked by Git. Alternatively, you can selectively manually commit only the relevant files (instead of "commit all changes") to the repository.
| test("should return false for an improper fraction", () =>{ | ||
| expect(isProperFraction(5, 2)).toEqual(false); | ||
| }); | ||
|
|
||
| // Case 3: Identify Negative Fractions: | ||
| test("should return true for negative proper fraction", () => ( | ||
| expect(isProperFraction(-4,7)).toEqual(true) | ||
| )); | ||
|
|
||
| // Case 4: Identify Equal Numerator and Denominator: | ||
| test("should return false when numerator equals denominator", () => { | ||
| expect(isProperFraction(3,3)).toEqual(false); | ||
| }); | ||
|
|
||
| // Stretch: | ||
| // What other scenarios could you test for? | ||
| test("should return true for negative proper fraction with negative denominator", () => { | ||
| expect(isProperFraction(4,-7)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
What are your expected range of numerators and denominators?
If you test all possible combinations of positive/negative numerator and denominator, you may discover a bug in your implementation.
| else if ( 2 <= rank && rank <= 9 ){ | ||
| return Number(rank); | ||
| } |
There was a problem hiding this comment.
Can you find out from AI the following?
What string value could make
2 <= rank && rank <= 9evaluates totruein JavaScript?
Can you figure out how to improve the checking condition to recognize only strings that represent valid ranks?
| test("should return numeric value for number cards (2-10)", () => { | ||
| const fiveofHearts = getCardValue("5♥"); | ||
| expect(fiveofHearts).toEqual(5); | ||
| }); | ||
| // Case 3: Handle Face Cards (J, Q, K): | ||
| test("should return 10 for face cards (J, Q, K) and 10", () => { | ||
| expect(getCardValue("10♦")).toEqual(10); |
There was a problem hiding this comment.
- 2 and 10 are good boundary cases to test.
- 10 is normally considered a number card.
| test("should return 11 for Ace (A)", () => { | ||
| expect(getCardValue("A♦")).toEqual(11); | ||
| }); |
There was a problem hiding this comment.
This test and the test on lines 7-9 are quite similar.
cjyuan
added
Reviewed
2 participants
made a new branch so it has no extra files