Skip to content
Open
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
37 changes: 27 additions & 10 deletions Sprint-1/fix/median.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
// Fix this implementation
// Start by running the tests for this function
// If you're in the Sprint-1 directory, you can run `npm test -- fix` to run the tests in the fix directory
function calculateMedian(list) {
// must be an array
if (!Array.isArray(list)) {
return null;
}

// Hint: Please consider scenarios when 'list' doesn't have numbers (the function is expected to return null)
// or 'list' has mixed values (the function is expected to sort only numbers).
// keep only VALID numeric values (exclude NaN, Infinity)
const numbers = list.filter(
(value) => typeof value === "number" && Number.isFinite(value)
);

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

// sort numbers without modifying original list
const sorted = [...numbers].sort((a, b) => a - b);
Comment thread
cjyuan marked this conversation as resolved.

const middle = Math.floor(sorted.length / 2);

// even length
if (sorted.length % 2 === 0) {
return (sorted[middle - 1] + sorted[middle]) / 2;
}

// odd length
return sorted[middle];
}

module.exports = calculateMedian;
module.exports = calculateMedian;
27 changes: 15 additions & 12 deletions Sprint-1/fix/median.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
// median.test.js

// Someone has implemented calculateMedian but it isn't
// passing all the tests...
// Fix the implementation of calculateMedian so it passes all tests

const calculateMedian = require("./median.js");

describe("calculateMedian", () => {
Expand All @@ -13,7 +7,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 +19,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 +29,9 @@ 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 +42,12 @@ 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))
);
});

// 🔥 EXTRA test for reviewer feedback
it("ignores NaN and Infinity values", () => {
expect(calculateMedian([1, 2, NaN, Infinity, 3])).toEqual(2);
});
});
14 changes: 13 additions & 1 deletion Sprint-1/implement/dedupe.js
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
function dedupe() {}
function dedupe(list) {
const result = [];

for (const item of list) {
if (!result.includes(item)) {
result.push(item);
}
}

return result; // ✅ always a NEW array
}

module.exports = dedupe;
43 changes: 30 additions & 13 deletions Sprint-1/implement/dedupe.test.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,44 @@
const dedupe = require("./dedupe.js");

/*
Dedupe Array

📖 Dedupe means **deduplicate**

In this kata, you will need to deduplicate the elements of an array

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

// 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");
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("given an array with no duplicates, it returns a new array", () => {
const input = [1, 2, 3];
const result = dedupe(input);

expect(result).toEqual([1, 2, 3]); // values match
expect(result).not.toBe(input); // 🔥 ensures it's a NEW array
});
Comment on lines 14 to +23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test should fail if the function returns the original array (instead of a copy of the original array).

The current test checks only if both the original array and the returned array contain identical elements.
In order to validate the returned array is a different array, we need an additional check.

Can you find out what this additional check is?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you address this comment?


// Given an array with numbers
// When passed to the dedupe function
// Then it should remove duplicate values
test("removes duplicate numbers", () => {
expect(dedupe([5, 1, 1, 2, 3, 2, 5, 8])).toEqual([5, 1, 2, 3, 8]);
});

// Given an array with strings
// When passed to the dedupe function
// Then it should remove duplicate values
test("removes duplicate strings", () => {
expect(dedupe(["a", "a", "a", "b", "b", "c"])).toEqual(["a", "b", "c"]);
});

// Given an array with strings or numbers
// Given an array with duplicates
// When passed to the dedupe function
// Then it should remove the duplicate values, preserving the first occurence of each element
// Then it should preserve the first occurrence order
test("preserves first occurrence order", () => {
expect(dedupe([1, 2, 1])).toEqual([1, 2]);
});
13 changes: 11 additions & 2 deletions Sprint-1/implement/max.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
function findMax(elements) {
function findMax(arr) {
let max = -Infinity;

for (let i = 0; i < arr.length; i++) {
if (typeof arr[i] === "number" && arr[i] > max) {
max = arr[i];
}
}

return max;
}

module.exports = findMax;
module.exports = findMax;
37 changes: 22 additions & 15 deletions Sprint-1/implement/max.test.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,50 @@
/* Find the maximum element of an array of numbers

In this kata, you will need to implement a function that find the largest numerical element of an array.

E.g. max([30, 50, 10, 40]), target output: 50
E.g. max(['hey', 10, 'hi', 60, 10]), target output: 60 (sum ignores any non-numerical elements)

You should implement this function in max.js, and add tests for it in this file.

We have set things up already so that this file can see your function from the other file.
*/

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");
test("given an empty array, returns -Infinity", () => {
expect(findMax([])).toBe(-Infinity);
});

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

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

// Given an array with just negative numbers
// When passed to the max function
// Then it should return the closest one to zero
test("given only negative numbers", () => {
expect(findMax([-10, -3, -50, -1])).toBe(-1);
});

// Given an array with decimal numbers
// When passed to the max function
// Then it should return the largest decimal number
test("given decimal numbers", () => {
expect(findMax([1.2, 5.7, 3.3])).toBe(5.7);
});

// 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("ignores non-number values", () => {
expect(findMax(["hey", 10, "hi", 60, 10])).toBe(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
// Then it should return the least surprising value
test("only non-number values returns -Infinity", () => {
expect(findMax(["a", "b", "c"])).toBe(-Infinity);
});
Comment on lines +41 to +50
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When a string representing a valid numeric literal (for example, "300") is compared to a number,
JavaScript first converts the string into its numeric equivalent before performing the comparison.
As a result, the expression 20 < "300" evaluates to true.

To test if the function can correctly ignore non-numeric values,
consider including a string such as "300" in the relevant test cases.


Why return a string (instead of a value of type "number") when the given array contains only non-number values?

When a function has a dual return type, it becomes unclear what the caller should expect. Developers would need to look at the implementation or documentation to understand the behavior.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you address this comment?

13 changes: 11 additions & 2 deletions Sprint-1/implement/sum.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
function sum(elements) {
function sum(arr) {
let total = 0;

for (let i = 0; i < arr.length; i++) {
if (typeof arr[i] === "number") {
total += arr[i];
}
}

return total;
}

module.exports = sum;
module.exports = sum;
33 changes: 20 additions & 13 deletions Sprint-1/implement/sum.test.js
Original file line number Diff line number Diff line change
@@ -1,36 +1,43 @@
/* Sum the numbers in an array

In this kata, you will need to implement a function that sums the numerical elements of an array

E.g. sum([10, 20, 30]), target output: 60
E.g. sum(['hey', 10, 'hi', 60, 10]), target output: 80 (ignore any non-numerical elements)
*/

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

// Acceptance Criteria:

// 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([])).toBe(0);
});

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

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

// Given an array with decimal/float numbers
// When passed to the sum function
// Then it should return the correct total sum
test("given decimal numbers", () => {
expect(sum([1.5, 2.5, 3])).toBe(7);
});
Comment on lines +27 to +29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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( 1.2 + 0.6 + 0.005 ).toEqual( 1.805 );                // This fail
  expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.8049999999999997 );   // This pass
  expect( 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?

Suggestion: Look up

  • Checking equality in floating point arithmetic in JavaScript
  • Checking equality in floating point arithmetic with Jest

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.

**Thanks for the feedback!

I’ve refactored the includes function to use a for...of loop as required.

I also fixed the test case for the empty array to ensure it correctly passes a target value.

All tests are now passing locally.**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This response does not address my request.

Note: This PR is not needed to complete Step 3. You don't have to complete this by the deadline.


// 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
// Then it should ignore the non-numerical values
test("ignores non-number values", () => {
expect(sum(["hey", 10, "hi", 60, 10])).toBe(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
// Then it should return the least surprising value
test("only non-number values returns 0", () => {
expect(sum(["a", "b", "c"])).toBe(0);
});
2 changes: 2 additions & 0 deletions Sprint-1/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 8 additions & 5 deletions Sprint-1/refactor/includes.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
// 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];
// handle non-array safely (optional but solid)
if (!Array.isArray(list)) {
return false;
}

for (const element of list) {
if (element === target) {
return true;
}
}

return false;
}

module.exports = includes;
module.exports = includes;
Loading
Loading