Skip to content
Closed
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
45 changes: 42 additions & 3 deletions Sprint-1/fix/median.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,49 @@
// 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).

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

// Ignore this Commit
// It is not my solution I got help with AI. I confess I need to undestand more those 3 methodos to think

Choose a reason for hiding this comment

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

I appreciate your honesty :D

//and get to this solution
// I will come back to it later.

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

//Filter non-numeric values and create a new array of only numbers

Choose a reason for hiding this comment

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

Generally we don't leave a line between the comment and the code the comment is related to

const numericList = list.filter(
(item) => typeof item === "number" && Number.isFinite(item)
);
Comment on lines +27 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you figure out a way to simplify the condition on line 28?


// If after filtering, there are no numbers left, return null
if (numericList.length === 0) {
return null;
}

//Sort the numeric list
const sortedList = [...numericList].sort((a, b) => a - b);
Copy link
Contributor

Choose a reason for hiding this comment

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

numericList is no longer the original array and it is not needed later. So we could improve the performance by not making another copy of the array by writing line 37 as

const sortedList = numericList.sort((a, b) => a - b);

Note: sortedList is a meaningful name so it is good to we keep it, but now sortedList and numericList refer to the the same array.

Choose a reason for hiding this comment

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

Can you explain why [...numericList] is needed? What would happen if I did numericList.sort instead?

Copy link
Author

Choose a reason for hiding this comment

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

because it is better not to change the original array in case I want to use it later in my code. I'm sorting a "copy" of it.


const middleIndex = Math.floor(sortedList.length / 2);

//Calculate the median based on list length (odd or even)
if (sortedList.length % 2 === 1) {
// Odd number of elements: return the middle element
return sortedList[middleIndex];
} else {
// Even number of elements: return the average of the two middle elements
const mid1 = sortedList[middleIndex - 1];
const mid2 = sortedList[middleIndex];
return (mid1 + mid2) / 2;
}
}

module.exports = calculateMedian;
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(arr) {
let result = [];

Choose a reason for hiding this comment

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

Does result have to be let here, or can it be const?
Same for item - can it be const?

Copy link
Author

Choose a reason for hiding this comment

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

I got it now. result should be const because the reference to the array doesn't need to be changed. For item in the , it can also be const, as a new item variable is declared for each iteration, because it's not reassigned within a single loop cycle.

for (let item of arr) {
if (!result.includes(item)) {
result.push(item);
}
}
return result;
}

console.log(dedupe(["a", "a", "a", "b", "b", "c"]));

module.exports = dedupe;
16 changes: 15 additions & 1 deletion Sprint-1/implement/dedupe.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,26 @@ 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("dedupe an empty array returns an empty array", () =>{
const inputEmpty = [];
const result = dedupe(inputEmpty);
expect(result).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", () => {

Choose a reason for hiding this comment

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

No need for the space between " and G

const inputNoDuplicates = ["a", "b", "c" ];
const result = dedupe(inputNoDuplicates);
expect(result).toEqual(["a", "b", "c"]);
});
Comment on lines 27 to +32
Copy link
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 the two arrays 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?


// 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(" Given an array with no duplicates", () => {
const inputMix = [1,1,2,2,3,3,4,4,5,5];
const result = dedupe(inputMix);
expect(result).toEqual([1,2,3,4,5]);
});
7 changes: 6 additions & 1 deletion Sprint-1/implement/max.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
function findMax(elements) {
function findMax(numbersList) {
let onlyNumbers = numbersList.filter((item) => typeof item === "number");
let currentMax = Math.max(...onlyNumbers); ;

Choose a reason for hiding this comment

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

Formatting: ensure all lines have the same indentation, and remove the double ;

Choose a reason for hiding this comment

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

Nice! Can you show me how you would do that with reduce as well?

return currentMax;
}
Comment on lines +1 to 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you improve the indentation?


console.log(findMax([1, "apple", 5, 0, "banana", 3]));

module.exports = findMax;
39 changes: 38 additions & 1 deletion Sprint-1/implement/max.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,65 @@ 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");
// This one is good to go!
test("given an empty array, returns -Infinity",() => {
const input = [];
const result = findMax(input);
expect(result).toEqual(-Infinity);
});

// Given an array with one number
// When passed to the max function
// Then it should return that number
test("given an array with 1 number, it returns the number ",() => {;
const inputMax1 = [3];
const result = findMax(inputMax1);

Choose a reason for hiding this comment

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

Formatting: ensure all lines have the same indentation. Same for all tests below

expect(result).toEqual(3);
});


// Given an array with both positive and negative numbers
// When passed to the max function
// Then it should return the largest number overall
test("give 2 positive numbers return the largest nunber",() => {
const inputMax2= [5,8];
const result = findMax(inputMax2);
expect(result).toEqual(8);
});

// Given an array with just negative numbers
// When passed to the max function
// Then it should return the closest one to zero
test ("Given an array with just negative numbers return colest to Zero",() =>{
const inputMax3= [-5,-3];
const result = findMax(inputMax3);
expect (result).toEqual(-3);
});

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

// 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("Given an array with non-number values,it should return the max and ignore non-numeric values ",() => {
const inputMax5 = [2, "apple", 5, "banana", 8, "watermelon"];
const result= findMax(inputMax5);
expect(result).toEqual(8);
});

// 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(" Given an array with only non-number values,it should return the least surprising value given how it behaves for all other inputs ", () => {

Choose a reason for hiding this comment

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

No space needed between " and G

const inputMax6= [ "computer", "bike", "car", "ball"];
const result= findMax(inputMax6);
expect(result).toEqual(-Infinity);
});

22 changes: 21 additions & 1 deletion Sprint-1/implement/sum.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,24 @@
function sum(elements) {
function sum(theArray) {

let onyNumbers = theArray.filter(item => typeof item === 'number');

Choose a reason for hiding this comment

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

Does this need to be let?

Copy link
Author

Choose a reason for hiding this comment

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

No, It can be const because the variable is assigned once with the filtered array.



Choose a reason for hiding this comment

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

Formatting:

  • Misspelling of onyNumbers
  • We usually don't leave two empty lines between code like this

const theFinalSum = onyNumbers.reduce((runningTotal, currentNumber) => {
return runningTotal + currentNumber;
}, 0);

return theFinalSum;
}

console.log (sum(["hey", 10, "hi", 60, 10]));

module.exports = sum;

//numbersList.filter((item) => typeof item === "number");
/* 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)
*/
33 changes: 32 additions & 1 deletion Sprint-1/implement/sum.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,55 @@ 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", () => {
const inputNumber = [];
const result = sum(inputNumber);
expect(result).toEqual(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 1 number, it returns the sam number", () => {

Choose a reason for hiding this comment

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

Misspelling: sam

const inputNumber = [34];
const result = sum(inputNumber);
expect(result).toEqual(34);
});

// 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 containing negative numbers",() => {
const inputNegative = [10, -5, 20, -15];
const result= sum(inputNegative )
expect (result).toEqual(10);
});

// 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/float numbers", () => {
const inputFloat = [10.2, 10.3];
const result = sum(inputFloat);
expect (result).toEqual(20.5);
});
Comment on lines +45 to +49
Copy link
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


// 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("Given an array containing non-number values", () => {
const inputNonNumbers = [1, "apple", 2, true, 3];
const result = sum(inputNonNumbers);
expect (result).toEqual(6)
});

// 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("Given an array with only non-number values" , () =>{
const nonNumbers = ["helo", "hi"];
const result = sum(nonNumbers);
expect (result).toEqual(0)
});
24 changes: 17 additions & 7 deletions Sprint-1/refactor/includes.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
// 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;
}
// function includes(list, target) {
// for (let index = 0; index < list.length; index++) {
// const element = list[index];
// if (element === target) {
// return true;
// }
// }
// return false;
// }


function includes(list,target){
for (let value of list) {

Choose a reason for hiding this comment

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

Does this need to be let?

Copy link
Author

Choose a reason for hiding this comment

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

I start to undestand now these const and let .. i thought let would always give me less risk of error, but I can see the opposite now.

thank you for your reviwes ;)

if (value === target) {
return true;
}
return false;
}
return false;
};
Comment on lines +14 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you improve the indentation?


module.exports = includes;
Loading