Skip to content

ZA-ITP-May-2025 | Christian Mayamba | Module-Data-Groups | Week 1 #573

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
32 changes: 29 additions & 3 deletions Sprint-1/fix/median.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,35 @@
// or 'list' has mixed values (the function is expected to sort only numbers).

function calculateMedian(list) {
const middleIndex = Math.floor(list.length / 2);
const median = list.splice(middleIndex, 1)[0];
return median;
if (!Array.isArray(list)) {
return null;
}

const numbersArray = list.filter((item) => typeof item === "number");
numbersArray.sort((a, b) => a - b);
const middleIndex = Math.floor(numbersArray.length / 2);

if (
list.every(
(item) => typeof item === "string" || item === null || item === undefined
)
) {
return null;
}
Comment on lines +17 to +23
Copy link
Member

Choose a reason for hiding this comment

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

This code looks like it wouldn't handle extra test-cases we could write, e.g. what would happen if the list contained only boolean values (true/false)?

We want to write code which is general - that is, it fixes the core problem statement, rather than a particular set of tests - please try to rewrite this so that it's more general.


if (numbersArray.length % 2 === 0) {
const tempMedian =
Copy link
Member

Choose a reason for hiding this comment

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

tempMedian isn't a very clear name for this variable - can you think of a more clear name?

numbersArray[middleIndex] + numbersArray[middleIndex - 1];
const median1 = tempMedian / 2;
return median1;
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you're calling this median1 rather than just median?

(And in fact, maybe you don't need a variable at all here - what value do you think the variable provides over just writing return tempMedian / 2;?)

} else {
const middleIndex = Math.floor(numbersArray.length / 2);
const median = numbersArray.splice(middleIndex, 1)[0];
Copy link
Member

Choose a reason for hiding this comment

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

This branch uses splice whereas the other branch doesn't - why are you getting elements differently in the two branches? What does splice do which is maybe useful or unhelpful?

return median;
}
}

module.exports = calculateMedian;

myArray = calculateMedian(["11", "mango"]);
console.log(myArray);
22 changes: 17 additions & 5 deletions Sprint-1/fix/median.test.js
Copy link

Choose a reason for hiding this comment

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

Tests passed, well done

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ describe("calculateMedian", () => {
{ 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 median for [${input}]`, () =>
expect(calculateMedian(input)).toEqual(expected))
);

[
Expand All @@ -24,7 +25,8 @@ describe("calculateMedian", () => {
{ input: [110, 20, 0], expected: 20 },
{ input: [6, -2, 2, 12, 14], expected: 6 },
].forEach(({ input, expected }) =>
it(`returns the correct median for unsorted array [${input}]`, () => expect(calculateMedian(input)).toEqual(expected))
it(`returns the correct median for unsorted array [${input}]`, () =>
expect(calculateMedian(input)).toEqual(expected))
);

it("doesn't modify the input array [3, 1, 2]", () => {
Expand All @@ -33,8 +35,17 @@ describe("calculateMedian", () => {
expect(list).toEqual([3, 1, 2]);
});

[ 'not an array', 123, null, undefined, {}, [], ["apple", null, undefined] ].forEach(val =>
it(`returns null for non-numeric array (${val})`, () => expect(calculateMedian(val)).toBe(null))
[
"not an array",
123,
null,
undefined,
{},
[],
["apple", null, undefined],
].forEach((val) =>
it(`returns null for non-numeric array (${val})`, () =>
expect(calculateMedian(val)).toBe(null))
);

[
Expand All @@ -45,6 +56,7 @@ describe("calculateMedian", () => {
{ 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))
it(`filters out non-numeric values and calculates the median for [${input}]`, () =>
expect(calculateMedian(input)).toEqual(expected))
);
});
18 changes: 17 additions & 1 deletion Sprint-1/implement/dedupe.js
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
function dedupe() {}
function dedupe(array) {
if (array.length === 0) {
return [];
}

const duplicates = array.filter(
(item, index) => array.indexOf(item) !== index
);

const set2 = new Set(duplicates);
return array.filter((item) => !set2.has(item));
Comment on lines +2 to +11
Copy link
Member

Choose a reason for hiding this comment

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

This feels like quite a complicated way of deduplicating an array. I'm also not quite sure it's behaving as the comments in the test file suggest. Can you give this another try? First focus on making sure you understand the requirements, then see if you can make it simpler.

}

testVar = [1, 20, "hi", 20, 6, 9, "hey"];
console.log(dedupe(testVar));

module.exports = dedupe;
10 changes: 9 additions & 1 deletion Sprint-1/implement/dedupe.test.js
Copy link

Choose a reason for hiding this comment

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

Tests passed, well done

Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,20 @@ E.g. dedupe([1, 2, 1]) target output: [1, 2]
// Given an empty array
// When passed to the dedupe function
// Then it should return an empty array
test.todo("given an empty array, it returns an empty array");
test("given an empty array, it returns an empty array", () => {
expect(dedupe([])).toEqual([]);
});

// Given an array with no duplicates
// When passed to the dedupe function
// Then it should return a copy of the original array
test("array with no duplicates, it returns a copy of the array", () => {
expect(dedupe([10, 20, 30])).toEqual([10, 20, 30]);
});

// Given an array with strings or numbers
// When passed to the dedupe function
// Then it should remove the duplicate values, preserving the first occurence of each element
test("array with strings and numbers, it should remove duplicates", () => {
expect(dedupe([10, "hi", "hey", 20, "hey", 30])).toEqual([10, "hi", 20, 30]);
Comment on lines 32 to +34
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this test does the same as the comment describes. The comment says "preserves the first occurence", but your test doesn't keep the first "hey".

});
16 changes: 16 additions & 0 deletions Sprint-1/implement/max.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
function findMax(elements) {
if (elements.length === 0) {
return "-Infinity";
}

if (elements.every((item) => typeof item === "string")) {
Copy link
Member

Choose a reason for hiding this comment

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

Same question as before - is "I can't find the max of an array of only strings" really the general statement here?

return "undefined";
Copy link
Member

Choose a reason for hiding this comment

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

Returning the string "undefined" seems like an odd choice here. Can you think of a more appropriate value to return?

}

const numbersArray = elements.filter((item) => typeof item === "number");
elements.sort((a, b) => a - b);
const maxNum = elements[elements.length - 1];

return maxNum;
}
const verArray = [-5, -10, -65, -6];

console.log(findMax(verArray));

module.exports = findMax;
22 changes: 21 additions & 1 deletion Sprint-1/implement/max.test.js
Copy link

Choose a reason for hiding this comment

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

Tests passed, well done

Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,48 @@ const findMax = require("./max.js");
// When passed to the max function
// Then it should return -Infinity
// Delete this test.todo and replace it with a test.
test.todo("given an empty array, returns -Infinity");
test("Empty array should return -Infinity", () => {
expect(findMax([])).toEqual("-Infinity");
});

// Given an array with one number
// When passed to the max function
// Then it should return that number
test("Array with number should return that number", () => {
expect(findMax([10])).toEqual(10);
});

// Given an array with both positive and negative numbers
// When passed to the max function
// Then it should return the largest number overall
test("Negative and positive numbers should return the largest overall", () => {
expect(findMax([10, -5, 58, -56])).toEqual(58);
});

// Given an array with just negative numbers
// When passed to the max function
// Then it should return the closest one to zero
test("Negative numbers should return the closest to zero", () => {
expect(findMax([-5, -10, -65, -6])).toEqual(-5);
});

// Given an array with decimal numbers
// When passed to the max function
// Then it should return the largest decimal number
test("Decimal numbers should return the largest one", () => {
expect(findMax([0.5, 10.5, 56.9, 3.5])).toEqual(56.9);
});

// Given an array with non-number values
// When passed to the max function
// Then it should return the max and ignore non-numeric values
test("Non-numeric values should be ignored and return the max numeric", () => {
expect(findMax(["hey", 10, "hi", 60, 10])).toEqual(60);
});

// Given an array with only non-number values
// When passed to the max function
// Then it should return the least surprising value given how it behaves for all other inputs
test("Only non-number values should return undefined", () => {
expect(findMax(["hey", "love", "hi", "hate"])).toEqual("undefined");
});
17 changes: 17 additions & 0 deletions Sprint-1/implement/sum.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
function sum(elements) {
if (elements.length === 0) {
return 0;
}

if (elements.every((item) => typeof item === "string")) {
Copy link
Member

Choose a reason for hiding this comment

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

Same questions as above

return "undefined";
}

const numbersArray = elements.filter((item) => typeof item === "number");

let sum1 = 0;
for (let i = 0; i < numbersArray.length; i++) {
sum1 += numbersArray[i];
}
return sum1;
}
testVar = [10, "jesus", 20, "hi"];
console.log(sum(testVar));

module.exports = sum;
19 changes: 18 additions & 1 deletion Sprint-1/implement/sum.test.js
Copy link

Choose a reason for hiding this comment

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

Tests passed, well done

Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,41 @@ const sum = require("./sum.js");
// Given an empty array
// When passed to the sum function
// Then it should return 0
test.todo("given an empty array, returns 0")
test("given an empty array, returns 0", () => {
expect(sum([])).toEqual(0);
});

// Given an array with just one number
// When passed to the sum function
// Then it should return that number
test("given array with one number, returns that number", () => {
expect(sum([10])).toEqual(10);
});

// Given an array containing negative numbers
// When passed to the sum function
// Then it should still return the correct total sum
test("given an array with negative numbers, returns the total sum", () => {
expect(sum([-10, -5, -50])).toEqual(-65);
});

// Given an array with decimal/float numbers
// When passed to the sum function
// Then it should return the correct total sum
test("given an array with decimal numbers, returns the total sum", () => {
expect(sum([10.5, 65.9, 50.8])).toEqual(127.2);
});

// Given an array containing non-number values
// When passed to the sum function
// Then it should ignore the non-numerical values and return the sum of the numerical elements
test("Non-numerical values should be ignored and return the sum of the rest", () => {
expect(sum(["hey", 10, "hi", 60, 10])).toEqual(80);
});

// Given an array with only non-number values
// When passed to the sum function
// Then it should return the least surprising value given how it behaves for all other inputs
test("Only non-number values should return undefined", () => {
expect(sum(["hey", "love", "hi", "hate"])).toEqual("undefined");
});