-
-
Notifications
You must be signed in to change notification settings - Fork 210
West Midlands | 25-ITP-Sept | Georgina Rogers | Sprint 1 | Sprint 1 Coursework #841
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
Conversation
… calculation and improve test descriptions
cjyuan
left a comment
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 check if any of this general feedback can help you further improve your code?
https://github.com/CodeYourFuture/Module-Data-Groups/blob/general-review-feedback/Sprint-1/feedback.md
Doing so can help reviewers speed up the review process. Thanks.
cjyuan
left a comment
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.
Changes look good.
|
|
||
| if (numbers.length === 0) return null; // Must have at least one number | ||
|
|
||
| const sorted = numbers.slice().sort((a, b) => a - b); |
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.
.slice() also creates a new array.
| if (!Array.isArray(elements) || elements.length === 0) { | ||
| return -Infinity; | ||
| } | ||
| let max = -Infinity; | ||
|
|
||
| for (const x of elements) { | ||
| if (typeof x === "number" && Number.isFinite(x)) { | ||
| if (x > max) max = x; | ||
| } | ||
| } | ||
|
|
||
| return max; | ||
| // removed .filter as this creates a new array so this avoids unnecessary clones | ||
|
|
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.
Indentation is off.
Learners, PR Template
Self checklist
Changelist
Functions and tests fixed, implemented, and refactored
Questions
.