ITP-Jan | NW | Jovy So | Module-Data-Groups | WEEK 1 #507
Conversation
| // const oddMiddleIndex = Math.floor(list.length / 2); | ||
| // const oddMedian = list.splice(oddMiddleIndex, 1)[0]; | ||
| const middleIndex = Math.floor(list.length / 2); | ||
| const median = list.splice(middleIndex, 1)[0]; | ||
| return median; | ||
| } | ||
| // const evenMiddleIndex = list.length / 2; | ||
| // const evenMedian = ((list.splice(evenMiddleIndex -1 ,1)[0]) + (list.splice(evenMiddleIndex ,1)[0])) / 2; | ||
| if (list.length % 2 === 1) return middleIndex ; | ||
| else return (list[middleIndex - 1] + list[middleIndex]) / 2; | ||
| }; | ||
|
|
There was a problem hiding this comment.
The arrays specified in median.test.js are already sorted by coincidence.
Typically, to find the median value of an array, we first need to sort it. However, to prevent modifying list directly, it's best to work with a cloned version instead.
Could you add one or more statements to create a cloned copy of list and then sort the numbers in the cloned array?
| function dedupe(elements) { | ||
| if (elements.length === 0) return []; | ||
| else if (elements.length === 1) return elements; | ||
|
|
||
| for (let i = elements.length -1; i >= 0; i--) { | ||
| for (let j = 0; j < i; j++) { | ||
| if (elements[j] === elements[i]) { | ||
| elements.splice(i, 1); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return elements; | ||
| }; |
There was a problem hiding this comment.
You can also consider using built-in functions to improve performance.
Some of the built-in functions/methods you can consider are Array.prototype.indexOf() and Array.prototype.includes().
| let numberOnly = elements.filter(item => typeof item === "number"); | ||
| // let nonNumber = elements.filter(item => typeof item !== "number"); | ||
| if (elements.length === 0) return -Infinity; | ||
| else if (elements.length === 1 && elements[0] > 0) return elements[0]; |
There was a problem hiding this comment.
With this condition, findMax(["10"]) would return "10", findMax([true]) would return true.
There was a problem hiding this comment.
You can also treat "an array with only non-number values" as an array of no numbers or an empty array.
| test("given an array with decimal/float number, returns correct total sum", () => { | ||
| expect(sum([1.1, 2.2, 3, 4.5])).toEqual(10.8); | ||
| }); |
There was a problem hiding this comment.
Decimal numbers in most programming languages (including JS) are internally represented in "floating point number" format. Floating point arithmetic is not exact. For example, the result of 46.5678 - 46 === 0.5678 is false because 46.5678 - 46 only yield a value that is very close to 0.5678. Even changing the order in which the program add/subtract numbers can yield different values.
So the following could happen
expect(sum([1.2, 0.6, 0.005])).toEqual(1.805); // This fail
expect(sum([1.2, 0.6, 0.005])).toEqual(1.8049999999999997); // This pass
expect(sum([0.005, 0.6, 1.2])).toEqual(1.8049999999999997); // This fail
console.log(1.2 + 0.6 + 0.005 == 1.805); // false
console.log(1.2 + 0.6 + 0.005 == 0.005 + 0.6 + 1.2); // false
Can you find a more appropriate way to test a value (that involves decimal number calculations) for equality?
| test("given an array with only non-number values, returns largest number", () => { | ||
| expect(findMax([true, "Apple", false, "Banana"])).toEqual(NaN); | ||
| }); |
There was a problem hiding this comment.
What do you mean by "the largest number"? NaN is "Not a number".
| if (elements.length === 0) return 0; | ||
| else if (elements.every(item => typeof item !== "number")) return NaN; | ||
| else if (elements.length === 1) return elements[0]; | ||
|
|
||
| let sumOfElements = 0; | ||
| for (let i = 0; i < elements.length; i++) { | ||
| if (typeof elements[i] === "number") { | ||
| sumOfElements += elements[i]; | ||
| } | ||
| } | ||
| return sumOfElements; | ||
| }; |
There was a problem hiding this comment.
You can also consider filter out all non-number values first, and then just work on the numbers in the filtered array.
cjyuan
added
Reviewed
|
It is a good practice to leave a brief description in every PR.
|
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.