Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 10 additions & 2 deletions Sprint-1/fix/median.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,16 @@
// 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];
let median = null;
if (Array.isArray(list) && !list.every((item) => typeof item != "number")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Excellent check of valid types for each item in an array. 👍
There are a few things to improve here:

You're doing a double negative check here. One positive check would reduce the complexity.

Use the early return method, where if your data doesn't match a certain criterion, you just stop with either a return or an error. More: https://gomakethings.com/the-early-return-pattern-in-javascript/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you so much, that was really helpful. I have implemented this now.

numericList = list.filter((item) => typeof item === "number");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmm, something about the declaration of these variables numericList and sortedList doesn't seem right. Think about the scope, think about mutability. What keywords should you use here to ensure your scope is proper, to avoid implicit globals?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh yes, I now understand the scope better after reading this. Thanks.

sortedList = numericList.toSorted((a, b) => a - b);
const middleIndex = Math.floor(sortedList.length / 2);
if (sortedList.length % 2 === 0) {
const medianArray = sortedList.splice(middleIndex - 1, 2);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmm. .splice() is a mutating operation. It changes the original array. The goal here is to access the value at a certain index of the array. In that regard, calculating the index would be a better solution for odd/even-length arrays. That will preserve the original array for later operations. Whereas right now, if you did more operations below, you'd be working with a mutated/changed array with fewer items in it.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, that's totally right. I have changed the code here to not use splice method and instead use the array index to access the values.

median = (medianArray[0] + medianArray[1]) / 2;
} else median = sortedList.splice(middleIndex, 1)[0];
}
return median;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice, organised return statement. If the above code falls through, this will catch it.
But question ❓ Can a median be null? Does it have to be a number? Or is null fine?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, it can't be null. I have added a check at the end for that. Let me know if it seems right. Thanks

}

Expand Down
22 changes: 17 additions & 5 deletions Sprint-1/fix/median.test.js
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))
);
});
13 changes: 12 additions & 1 deletion Sprint-1/implement/dedupe.js
Original file line number Diff line number Diff line change
@@ -1 +1,12 @@
function dedupe() {}
function dedupe(arr) {
if (arr.length === 0) return arr;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice length check here 👍 But could the array just not be present, i.e. null/undefined?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh yes, I thought about these null/ undefined checks when working on these test files, but as this case was not described already, I thought we might not be supposed to do this.

But I think it is always a good practice to have those checks. I have added them now. Kindly check, please.

else {
dedupeArray = [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if dedupeArray is not declared before, you should first declare it so that it's scoped properly. I've left a link in a similar comment above for some research.

arr.forEach((element) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is generally good logic. Well done 👍
However, if you want a much simpler approach, a better data structure would be a Set. You'd create a set from this array, then return an array of the set.
that'd do this for you.
https://www.geeksforgeeks.org/javascript/how-to-convert-set-to-array-in-javascript/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for introducing Set into my life. I am using them now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hehe, happy to do so. But ensure you follow the goals of the current chapter. There's always another advanced technique, but follow the goal/description of the task, even if it's painful to do so.

if (!dedupeArray.includes(element)) dedupeArray.push(element);
});
return dedupeArray;
}
}

module.exports = dedupe;
51 changes: 37 additions & 14 deletions Sprint-1/implement/dedupe.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,41 @@ E.g. dedupe(['a','a','a','b','b','c']) target output: ['a','b','c']
E.g. dedupe([5, 1, 1, 2, 3, 2, 5, 8]) target output: [5, 1, 2, 3, 8]
E.g. dedupe([1, 2, 1]) target output: [1, 2]
*/
describe("dedupe", () => {
// Given an empty array
// When passed to the dedupe function
// Then it should return an empty array
it("given an empty array it should return an empty array", () => {
const array = [];
dedupeArray = dedupe(array);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this one slipped through the crack. Let's add a keyword

expect(dedupeArray).toEqual([]);
});

// Acceptance Criteria:

// 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");

// Given an array with no duplicates
// When passed to the dedupe function
// Then it should return a copy of the original array

// 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
// Given an array with no duplicates
// When passed to the dedupe function
// Then it should return a copy of the original array
[["a", "b", "c"], ["A", 1, "j", "?"], ["c"]].forEach((val) =>
it(`returns copy of the original array if there are no duplicates in [${val}]`, () =>
expect(dedupe(val)).toEqual(val))
);
// Given an array with strings or numbers
// When passed to the dedupe function
// Then it should remove the duplicate values, preserving the first occurrence of each element
[
{ input: [1, 2, 1, 3, 1, 2, 10, 5, 0, 10], expected: [1, 2, 3, 10, 5, 0] },
{ input: [1, 2, 1, 4], expected: [1, 2, 4] },
{ input: [1, 1, 1, 1, 1], expected: [1] },
{
input: ["banana", "apple", "apple", "banana", "apple", "banana"],
expected: ["banana", "apple"],
},
{
input: [" ", "empty", "", " ", "", "empty"],
expected: [" ", "empty", ""],
},
{ input: ["2", "2", "3", "1"], expected: ["2", "3", "1"] },
].forEach(({ input, expected }) =>
it(`returns a copy of array removing the duplicates from [${input}]`, () =>
expect(dedupe(input)).toEqual(expected))
);
});
4 changes: 3 additions & 1 deletion Sprint-1/implement/max.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
function findMax(elements) {
function findMax(array) {
numbersArray = array.filter((value) => typeof value === "number");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Excellent type check. But also ensure to do null/undefined check

return Math.max(...numbersArray);
}

module.exports = findMax;
122 changes: 93 additions & 29 deletions Sprint-1/implement/max.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,32 +12,96 @@ We have set things up already so that this file can see your function from the o

const findMax = require("./max.js");

// Given an empty array
// 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");

// Given an array with one number
// When passed to the max function
// Then it should return that number

// Given an array with both positive and negative numbers
// When passed to the max function
// Then it should return the largest number overall

// Given an array with just negative numbers
// When passed to the max function
// Then it should return the closest one to zero

// Given an array with decimal numbers
// When passed to the max function
// Then it should return the largest decimal number

// Given an array with non-number values
// When passed to the max function
// Then it should return the max and ignore non-numeric values

// 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
describe("findMax", () => {
// Given an empty array
// When passed to the max function
// Then it should return -Infinity
it("if an empty array is passed to the the findMax function, -Infinity should be returned", () => {
emptyArray = [];
expect(findMax(emptyArray)).toEqual(-Infinity);
});

// Given an array with one number
// When passed to the max function
// Then it should return that number
[[1], [70], [0], [-25], [100129]].forEach((val) =>
it(`When an array with only one number is passed i.e. [${val}], it should return that number`, () =>
expect(findMax(val)).toEqual(val[0]))
);

// Given an array with both positive and negative numbers
// When passed to the max function
// Then it should return the largest number overall
[
{ input: [12, -2, 4, 6, 0], expected: 12 },
{ input: [2, 5, 990, -4], expected: 990 },
{ input: [0, -1, -5], expected: 0 },
{ input: [0, -1, 300, 3], expected: 300 },
].forEach(({ input, expected }) =>
it(`returns the max number from the array [${input}]`, () =>
expect(findMax(input)).toEqual(expected))
);

// Given an array with just negative numbers
// When passed to the max function
// Then it should return the closest one to zero
[
{ input: [-4, -2, -1902, -2, -1], expected: -1 },
{ input: [-9088, -9087, -990788, -4888777], expected: -9087 },
{ input: [-1, -5], expected: -1 },
].forEach(({ input, expected }) =>
it(`returns the max number from the array of negative numbers only [${input}]`, () =>
expect(findMax(input)).toEqual(expected))
);

// Given an array with decimal numbers
// When passed to the max function
// Then it should return the largest decimal number
[
{ input: [-4.2, 2.8, -19.8009, 2.4, 1.5], expected: 2.8 },
{ input: [-90.88, 0.001, -990.788], expected: 0.001 },
{ input: [-1.11, -5.3], expected: -1.11 },
].forEach(({ input, expected }) =>
it(`returns the max number from the array of decimal numbers [${input}]`, () =>
expect(findMax(input)).toEqual(expected))
);

// Given an array with non-number values
// When passed to the max function
// Then it should return the max and ignore non-numeric values
[
{ input: [-4, -2, "what is this", -1902, -2, "??", -1], expected: -1 },
{
input: [-9088, ".oi9e9", "1000000", -9087, 990788, -4888777],
expected: 990788,
},
{ input: [-1.11, "here", -5.233, "ignore me please"], expected: -1.11 },
].forEach(({ input, expected }) =>
it(`returns the max number from the array of numbers and non-numbers values [${input}]`, () =>
expect(findMax(input)).toEqual(expected))
);

// 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

/* Ans: If there is no number in the array than considering the behavior for the above inputs
the least surprising value in return would be -Infinity as our array has zero elements which are numbers*/
[
{
input: ["Not a number", "what is this", "least surprising", "??", "what"],
expected: -Infinity,
},
{
input: ["kkdkas", "Ahan!", "23"],
expected: -Infinity,
},
{
input: ["here", "Least surprising", "????", "vale is", "-Infinity"],
expected: -Infinity,
},
].forEach(({ input, expected }) =>
it(`returns the least surprising value for only non-numbers array [${input}]`, () =>
expect(findMax(input)).toEqual(expected))
);
});
8 changes: 7 additions & 1 deletion Sprint-1/implement/sum.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
function sum(elements) {
function sum(array) {
numbersArray = array.filter((value) => typeof value === "number");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same here, null/undefined check should be added

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, look into .reduce() after this section is over.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, I read about reduce, and it seems better to use that here.

let sum = 0;
numbersArray.forEach((element) => {
sum += element;
});
return sum;
}

module.exports = sum;
86 changes: 64 additions & 22 deletions Sprint-1/implement/sum.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,71 @@ E.g. sum(['hey', 10, 'hi', 60, 10]), target output: 80 (ignore any non-numerical

const sum = require("./sum.js");

// Acceptance Criteria:
describe("sum", () => {
// Given an empty array
// When passed to the sum function
// Then it should return 0
it("0 should be return when an empty array is passed", () =>
expect(sum([])).toEqual(0));

// Given an empty array
// When passed to the sum function
// Then it should return 0
test.todo("given an empty array, returns 0")
// Given an array with just one number
// When passed to the sum function
// Then it should return that number
[[1], [22], [-29], [0]].forEach((input) =>
it(`an array with just one number [${input}] should return that number`, () =>
expect(sum(input)).toEqual(input[0]))
);

// Given an array with just one number
// When passed to the sum function
// Then it should return that number
// Given an array containing negative numbers
// When passed to the sum function
// Then it should still return the correct total sum
[
{ input: [-1, -2, -100], expected: -103 },
{ input: [-26, 0, -90], expected: -116 },
{ input: [20, -20, 0, 45], expected: 45 },
].forEach(({ input, expected }) =>
it(`array containing negative numbers [${input}] should return the correct sum`, () =>
expect(sum(input)).toEqual(expected))
);

// Given an array containing negative numbers
// When passed to the sum function
// Then it should still return the correct total sum
// Given an array with decimal/float numbers
// When passed to the sum function
// Then it should return the correct total sum
[
{ input: [1.1, 2.222, 100.001], expected: 103.32300000000001 },
{ input: [26.55, 0.001, -90.34], expected: -63.789 },
{ input: [-0.01, 45.9], expected: 45.89 },
].forEach(({ input, expected }) =>
it(`array containing decimal/float numbers [${input}] should return the correct sum`, () =>
expect(sum(input)).toEqual(expected))
);
// 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
[
{
input: [1.1, "what", 2.222, "is", "this???", " ", 100.001],
expected: 103.32300000000001,
},
{ input: [26.55, "Okay", "!!", 0.001, -90.34], expected: -63.789 },
{ input: [1, "12", 45], expected: 46 },
].forEach(({ input, expected }) =>
it(`array containing non-number values [${input}] should ignore them and return the correct sum of numbers in it`, () =>
expect(sum(input)).toEqual(expected))
);

// Given an array with decimal/float numbers
// When passed to the sum function
// Then it should return the correct total sum

// 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

// 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
// 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
[
{
input: ["what", "is", "this???", " "],
expected: 0,
},
{ input: ["26.55", "Okay", "!!"], expected: 0 },
{ input: ["write", "whatever", "%-959"], expected: 0 },
].forEach(({ input, expected }) =>
it(`array containing only non-number values [${input}] should should return 0`, () =>
expect(sum(input)).toEqual(expected))
);
});
7 changes: 2 additions & 5 deletions Sprint-1/refactor/includes.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Refactor the implementation of includes to use a for...of loop

function includes(list, target) {
for (let index = 0; index < list.length; index++) {
const element = list[index];
if (element === target) {
return true;
}
for (const element of list) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, you must use a for-loop, I see. Otherwise, .includes() would be fine. Poor thing 🤣

if (element === target) return true;
}
return false;
}
Expand Down
Loading
Loading