Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions Sprint-1/fix/median.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,33 @@
// }

// Explanation:
// .splice() is a mutating method, so it updates the array in place.
// to prevent this, you could take a copy of the array and use .splice(),
// or you could use bracket notation to element at the desired index
// The original implementation has several mistakes:
// 1 - .splice() mutates or changes the original array. We don't want this to happen.
// 2 - We actually don't need .splice() or any other method to find a median.
// 3 - We must sort the array before we search for the median.
// 4 - We should check if the 'list' parameter actually has numbers, otherwise try to filter
// out all non-numeric values and throw an error or return null if there are no numeric values.

function calculateMedian(list) {
const middleIndex = Math.floor(list.length / 2);

if (list.length % 2 === 0) { // use the remainder operator (%) to check if the array length is even
return (list[middleIndex - 1] + list[middleIndex]) / 2;
// Example of a function calculating median and returning null in case no numeric values provided.

function calculateMedian(list) {
if (!Array.isArray(list)) {
return null;
}
const onlyNumberList = list.filter(item => typeof item === 'number' && !isNaN(item));

return list[middleIndex];
if (!onlyNumberList.length) {
return null;
} else {
const sortedList = onlyNumberList.sort((current, next) => current - next);
const indexNearMiddleOfArray = Math.floor(sortedList.length / 2);
if (sortedList.length % 2 === 0) {
return (sortedList[indexNearMiddleOfArray - 1] + sortedList[indexNearMiddleOfArray]) / 2;
} else {
return sortedList[indexNearMiddleOfArray];
}
}
}

module.exports = calculateMedian;
44 changes: 33 additions & 11 deletions Sprint-1/fix/median.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,42 @@
const calculateMedian = require("./median.js");

describe("calculateMedian", () => {
it("returns the median for odd length array", () => {
expect(calculateMedian([1, 2, 3])).toEqual(2);
expect(calculateMedian([1, 2, 3, 4, 5])).toEqual(3);
});
[
{ input: [1, 2, 3], expected: 2 },
{ input: [1, 2, 3, 4, 5], expected: 3 },
{ input: [1, 2, 3, 4], expected: 2.5 },
{ input: [1, 2, 3, 4, 5, 6], expected: 3.5 },
].forEach(({ input, expected }) =>
it(`returns the median for [${input}]`, () => expect(calculateMedian(input)).toEqual(expected))
);

it("returns the average of middle values for even length array", () => {
expect(calculateMedian([1, 2, 3, 4])).toEqual(2.5);
expect(calculateMedian([1, 2, 3, 4, 5, 6])).toEqual(3.5);
});
[
{ input: [3, 1, 2], expected: 2 },
{ input: [5, 1, 3, 4, 2], expected: 3 },
{ input: [4, 2, 1, 3], expected: 2.5 },
{ input: [6, 1, 5, 3, 2, 4], expected: 3.5 },
].forEach(({ input, expected }) =>
it(`returns the correct median for unsorted array [${input}]`, () => expect(calculateMedian(input)).toEqual(expected))
);

it("doesn't modify the input", () => {
it("doesn't modify the input array [1, 2, 3]", () => {
const list = [1, 2, 3];
calculateMedian(list);

expect(list).toEqual([1, 2, 3]);
});
});

[ 'not an array', 123, null, undefined, {}, [], ["apple", null, undefined] ].forEach(val =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add these tests to the main branch too, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, the main should also be updated. I will first update the tests as to my comment above - not to expect throw and after we merge these changes into solutions, I will create similar change to main.

it(`returns null for non-numeric array (${val})`, () => expect(calculateMedian(val)).toBe(null))
);

[
{ input: [1, 2, "3", null, undefined, 4], expected: 2 },
{ input: ["apple", 1, 2, 3, "banana", 4], expected: 2.5 },
{ input: [1, "2", 3, "4", 5], expected: 3 },
{ input: [1, "apple", 2, null, 3, undefined, 4], expected: 2.5 },
{ input: [3, "apple", 1, null, 2, undefined, 4], expected: 2.5 },
{ input: ["banana", 5, 3, "apple", 1, 4, 2], expected: 3 },
].forEach(({ input, expected }) =>
it(`filters out non-numeric values and calculates the median for [${input}]`, () => expect(calculateMedian(input)).toEqual(expected))
);
});