London | 26-ITP-JAN | Asha Ahmed | Sprint 1 | Data Groups#1031
London | 26-ITP-JAN | Asha Ahmed | Sprint 1 | Data Groups#1031ashaahmed7 wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
…prove median calculation logic
…ty array and unique values
…, handling non-numeric values and empty arrays.
…on-numeric values
… array, handling empty arrays and non-array inputs.
ashaahmed7
added
the
Needs Review
| const middleIndex = Math.floor(list.length / 2); | ||
| const median = list.splice(middleIndex, 1)[0]; | ||
| if (!Array.isArray(list)) return null; | ||
| const num = list.filter((l) => typeof l === "number").sort((a, b) => a - b); |
There was a problem hiding this comment.
-
Do you plan to include
-Infinity,Infinity, andNaNin the median calculation (and also in the function inimplement/sum.js)? -
Naming convention for variables that stores an array is:
- Use plural form (e.g.
numbers), or - Append a suffix
ArrayorListto the name (e.g.,numberArray)
- Use plural form (e.g.
There was a problem hiding this comment.
I currently filter values using typeof l === "number", which includes Infinity, -Infinity, and NaN.
However, these values can distort the median calculation (especially NaN, which propagates).
I plan to refine the filter to include only finite numbers using Number.isFinite() for more robust behaviour.
const numbers = list
.filter((l) => Number.isFinite(l))
.sort((a, b) => a - b);
| if (numericElements.length === 0) { | ||
| return -Infinity; | ||
| } |
There was a problem hiding this comment.
Should the function preserve this characteristic?
sum(["apple", "banana", 1, 10]) == sum(["apple", "banana"]) + sum([1, 10])
There was a problem hiding this comment.
Good question — with the current implementation, this property does not hold because the function returns -Infinity when no numeric values are present.
This causes inconsistencies when combining results (e.g., -Infinity + 11 = -Infinity).
If we want the function to preserve this additive property, it would be better to return 0 when no numeric values are found, since 0 is the neutral element for addition.
I can update the function accordingly depending on the intended behaviour.
if (numericElements.length === 0) {
return 0;
}
There was a problem hiding this comment.
The spec only mentions "least surprising" value. It is up to you what's that value should be.
I can only provide you with additional info to make informed decision.
cjyuan
added
Reviewed
ashaahmed7
added
the
Needs Review
|
Your explanation sounds good to me. |
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Completed the exercises in each directory on Sprint 1
Questions
N/A