-
-
Notifications
You must be signed in to change notification settings - Fork 195
Glasgow | May-2025 | Adiyah Farhan | Sprint-3 #573
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
base: main
Are you sure you want to change the base?
Glasgow | May-2025 | Adiyah Farhan | Sprint-3 #573
Conversation
…tory-rewrite/1-get-angle -type.js
…test.js using Jest.
… string, and tested it for different test cases with jest
…ordinal, also tested it with different test cases in jest
…sible test cases with jest
…d password and tested for each case using jest
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.
Overall, very well done. You've written good solutions and good test cases. I have left a couple of comments for some of them for you to take a look at.
if (numerator < denominator) return true; | ||
if (Math.abs(numerator) < denominator) return true; | ||
else if (Math.abs(numerator) > denominator) return false; | ||
else return false; | ||
} |
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 see any way you could remove the extra else branch here?
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.
Does this work when the card value is 10?
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.
As a matter of practice, you might consider implementing repeat() yourself, rather than using the built-in string .repeat()
function
Thank you @LonMcGregor for taking the time to review my pull request and provide valuable suggestions. I have implemented the recommended changes and committed them to the respective files. Kindly review the updates at your convenience. |
Hi adiyah, you've correctly identified the ways to improve your solution, and good job doing the full repeat implementation. This sprint is complete now, you can close it and move to the next |
Learners, PR Template
Self checklist
Changelist
Files from the following folders
1-key-implement
2-mandatory-rewrite
3-mandatory practice/implement
has been updated/implemented as per the requirements.
One file passwordValidator from 4-stretch-investigate has implemented.
Questions
Not yet