Manchester | 26-ITP-May | Abdu Hassen | Sprint 1 | Data Groups - #1257
Manchester | 26-ITP-May | Abdu Hassen | Sprint 1 | Data Groups#1257Abduhasen wants to merge 3 commits into
Conversation
Abduhasen
added
📅 Sprint 1
hey-hammad
added
Review in progress
hey-hammad
left a comment
hey-hammad
left a comment
There was a problem hiding this comment.
Nice effort overall, please review the comments and fix the code quality issues, Also several tests are duplicated and share identical descriptions, and some test names could be made clearer. Consider adding edge cases for NaN and arrays containing a single non-numeric value.
| if (!Array.isArray(list)) { | ||
| return null; | ||
| } | ||
| list = list.filter((item) => typeof item === "number"); |
There was a problem hiding this comment.
It would be better to create a new variable so the filter operation doesn't modify the input(original array)
also please review the following for a more robust validation.
| return null; | ||
| } | ||
| list = list.filter((item) => typeof item === "number"); | ||
| if (list.length === 0) { |
There was a problem hiding this comment.
Its best to put all the guard clauses at start of the function.
https://medium.com/@timothydan/javascript-guard-clauses-64b999e3240
| } else if (elements.length === 1) { | ||
| return elements[0]; | ||
| } | ||
| const number = elements.filter((value) => typeof value === "number"); |
There was a problem hiding this comment.
please review the following for a more robust validation.
| function findMax(elements) { | ||
| if (elements.length === 0) { | ||
| return Infinity; | ||
| } else if (elements.length === 1) { |
There was a problem hiding this comment.
What will happen if there is only a single item in the array and that item is a string?
| @@ -1,4 +1,20 @@ | |||
| function findMax(elements) { | |||
| if (elements.length === 0) { | |||
| return Infinity; | |||
There was a problem hiding this comment.
This should not be Infinity
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/max
hey-hammad
added
Reviewed
Abduhasen
added
Needs Review
hey-hammad
added
Review in progress
| if (!Array.isArray(list)) { | ||
| return null; | ||
| } | ||
| filteredList = list.filter((item) => typeof item === "number"); |
There was a problem hiding this comment.
-
What happens if a variable is assigned without a declaration in JavaScript?
-
What would happen if you pass the following input to the function?
[1, 2, 3,NaN, 4, 5]
| if (elements.length === 0) { | ||
| return 0; | ||
| } | ||
| const number = elements.filter((value) => typeof value === "number"); |
There was a problem hiding this comment.
What would happen if you pass the following input to the function?
[1, 2, 3,NaN, 4, 5]
hey-hammad
added
Reviewed
Abduhasen
added
Needs Review
hey-hammad
added
Review in progress
2 participants
Learners, PR Template
Self checklist
Changelist
-writing test cases for a function
Questions
N/A