WM | 25-ITP-May | Nahom Mesfin | Sprint 3 coursework #677
Conversation
nahommesfin77
added
the
Needs Review
LonMcGregor
added
Review in progress
| @@ -9,6 +9,7 @@ | |||
|
|
|||
| function isProperFraction(numerator, denominator) { | |||
| if (numerator < denominator) return true; | |||
There was a problem hiding this comment.
What happens if I call isProperFraction(-70,5)?
-70/5 is not a proper fraction.
There was a problem hiding this comment.
The assertion will fail because i have not used aboslute value Maths.abs in my function to determine if a negative fractioon is really a proper fraction or not. I have fixed and changed the condition to return absolute values.
line 11
- I have change the if conditon to
return Math.abs(numerator) < Math.abs(denominator);
| if (rank === "A") return 11; | ||
| if (rank >= '2' && rank <= '9') return Number(rank); |
There was a problem hiding this comment.
What happens if I call getCardValue("222♥")?
There was a problem hiding this comment.
Yes, it should have thrown an error, but because of string comparison of "222" and "2" it will pass the condition and logs 222. I have fixed this issue by using regular expression.
- Updated line 13
if (/^[2-9]$/.test(rank)) return Number(rank);
| @@ -1,5 +1,11 @@ | |||
| function countChar(stringOfCharacters, findCharacter) { | |||
| return 5 | |||
| let count = 0; | |||
There was a problem hiding this comment.
This is very minor - try to keep the indentation the same for all the code. here your first declaration is indented further.
There was a problem hiding this comment.
Ok, I will. thanks for noticing, I have adjusted the identation.
| @@ -1,5 +1,8 @@ | |||
| function getOrdinalNumber(num) { | |||
There was a problem hiding this comment.
In english we would say "21st" - can you make your code work in these general cases too?
There was a problem hiding this comment.
I have updated the function to include cases like 21st, 22nd and so on. I have used a modulo operator to get the last digits of the numbers num % 10; and num % 100; and used them to determine the cases for each number.
- Updated line 2 to 10
| function repeat() { | ||
| return "hellohellohello"; | ||
| function repeat(str, count) { | ||
| return str.repeat(count); |
There was a problem hiding this comment.
What about your error condition you are testing for?
There was a problem hiding this comment.
Yes, i should have added an error conditon to throw an error. I have added an error that throws a negative integer.
- line 2 - 4
if (count < 0) { throw new Error("Count must be a non-negative integer."); }
LonMcGregor
added
Reviewed
nahommesfin77
added
the
Needs Review
|
Great work making these changes - you are done with this sprint now |
LonMcGregor
added
Complete
nahommesfin77
removed
the
Reviewed
Learners, PR Template
Self checklist
Changelist
In Implement, I wrote functions step-by-step with assertions to cover different cases. In Rewrite, I converted previous assertions to use Jest syntax for clearer and easier testing. In Practice I built and ran tests using Jest and vscode's code runner to ensure all functions meet requirements.