-
-
Notifications
You must be signed in to change notification settings - Fork 275
London | 26-ITP-JAN | Asha Ahmed | Sprint 1 | Data Groups #1031
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?
Changes from all commits
b8eab36
2c8ec96
ba5eb53
935e222
7b98200
084addb
065f1e6
c01c0bc
436abd9
79895f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,22 @@ | ||
| function dedupe() {} | ||
| function dedupe(input) { | ||
| if (!Array.isArray(input)) { | ||
| return null; | ||
| } | ||
|
|
||
| if (input.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const newArray = []; | ||
|
|
||
| for (let i = 0; i < input.length; i++) { | ||
| const element = input[i]; | ||
|
|
||
| if (!newArray.includes(element)) { | ||
| newArray.push(element); | ||
| } | ||
| } | ||
| return newArray; | ||
| } | ||
|
|
||
| module.exports = dedupe; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,9 @@ | ||
| function findMax(elements) { | ||
| const numberElements = elements.filter( | ||
| (el) => typeof el === "number" && !Number.isNaN(el) | ||
| ); | ||
| if (numberElements.length === 0) return -Infinity; | ||
| return Math.max(...numberElements); | ||
| } | ||
|
|
||
| module.exports = findMax; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,22 @@ | ||
| function sum(elements) { | ||
| if (!Array.isArray(elements)) { | ||
| return null; | ||
| } | ||
| // Check if array is empty | ||
| if (elements.length === 0) { | ||
| return 0; | ||
| } | ||
| // Filter only numeric values | ||
| const numericElements = elements.filter((item) => typeof item === "number"); | ||
| if (numericElements.length === 0) { | ||
| return -Infinity; | ||
| } | ||
|
Comment on lines
+11
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the function preserve this characteristic?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) {
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec only mentions "least surprising" value. It is up to you what's that value should be. |
||
| // Calculate the sum of numeric values | ||
| let total = 0; | ||
| for (let i = 0; i < numericElements.length; i++) { | ||
| total += numericElements[i]; | ||
| } | ||
| return total; | ||
| } | ||
|
|
||
| module.exports = sum; | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
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:
numbers), orArrayorListto the name (e.g.,numberArray)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.
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);