-
-
Notifications
You must be signed in to change notification settings - Fork 326
Manchester | 26-ITP-May | Yee Man Tsang | Sprint 1 | Data Group Coursework #1359
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,33 @@ | |
| // 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)) { | ||
| // [...list] creates a shallow copy | ||
| const listNumberOnlyAsc = [...list].filter( | ||
| (item) => !isNaN(item) && item !== null | ||
| ); | ||
| if (listNumberOnlyAsc == undefined || listNumberOnlyAsc == 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will the condition on line 14 ever be |
||
| return null; | ||
| } | ||
| const listPureNumber = [...listNumberOnlyAsc].filter( | ||
| (item) => typeof item == "number" | ||
| ); | ||
|
Comment on lines
+17
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const listSorted = [...listPureNumber].sort((a, b) => a - b); | ||
|
|
||
| if (listSorted.length % 2 === 1) { | ||
| const middleIndex = Math.floor(listSorted.length / 2); | ||
| const median = [...listSorted].splice(middleIndex, 1)[0]; //code '.splice(middleIndex,1)' means remove the middle index num in array and put it in a new array, '[0]' is the first item in this new array | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use the same syntax you are using on line 30? |
||
| return median; | ||
| } else { | ||
| //retrieve the second middle number from the 2 middle numbers, and use it to retrieve the first middle number amd calculate the median | ||
| const secondMiddleIndex = Math.floor(listSorted.length / 2); | ||
| const median = | ||
| (listSorted[secondMiddleIndex - 1] + listSorted[secondMiddleIndex]) / 2; | ||
| return median; | ||
| } | ||
| } else { | ||
| return null; | ||
| } | ||
|
Comment on lines
+33
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also express as This way, we don't need to use |
||
| } | ||
|
|
||
| module.exports = calculateMedian; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,11 @@ | ||
| function dedupe() {} | ||
| function dedupe(list) { | ||
| if (list.length === 0){ | ||
| return null; | ||
| }else{ | ||
|
Comment on lines
+2
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spacing around |
||
| //compare the array elements and check is there is duplicate | ||
| listDedupe = list.filter((item, index) => list.indexOf(item) === index); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return listDedupe; | ||
| } | ||
| } | ||
|
|
||
| module.exports = dedupe; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,13 +16,33 @@ E.g. dedupe([1, 2, 1]) returns [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"); | ||
| describe("dedupe", () => { | ||
| it("returns empty array", () => { | ||
| expect(dedupe([])).toBe(null) | ||
| }); | ||
|
|
||
| // Given an array with no duplicates | ||
| // When passed to the dedupe function | ||
| // Then it should return a copy of the original array | ||
| [ | ||
| { input: ['a','b','c','d','e','f'], expected: ['a','b','c','d','e','f'] }, | ||
| { input: [1, 2, 3, 4, 5, 6], expected: [1, 2, 3, 4, 5, 6] }, | ||
| { input: ['apple', 'orange', 'grape'], expected: ['apple', 'orange', 'grape'] }, | ||
| { input: [12, 23, 34, 45, 56, 67], expected: [12, 23, 34, 45, 56, 67] }, | ||
| ].forEach(({ input, expected }) => | ||
| it(`returns the original list with no duplicates [${input}]`, () => expect(dedupe(input)).toEqual(expected)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current test checks only if both the original array and the returned array contain identical elements. Can you find out what this additional check is? |
||
| ); | ||
|
|
||
| // Given an array of strings or numbers | ||
| // When passed to the dedupe function | ||
| // Then it should return a new array with duplicates removed while preserving the | ||
| // first occurrence of each element from the original array. | ||
| [ | ||
| { input: ['a','a','a','b','b','c'], expected: ['a','b','c'] }, | ||
| { input: [5, 1, 1, 2, 3, 2, 5, 8], expected: [5, 1, 2, 3, 8] }, | ||
| { input: [1, 2, 1], expected: [1, 2] }, | ||
| { input: ['apple', 'banana', 'apple', 'banana'], expected: ['apple', 'banana'] }, | ||
| ].forEach(({ input, expected }) => | ||
| it(`returns the deduplicated list [${input}]`, () => expect(dedupe(input)).toEqual(expected)) | ||
| ); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,18 @@ | ||
| function findMax(elements) { | ||
| const elementsNumberOnly = [...elements].filter((element) => !isNaN(element)); | ||
| const elementsPureNumber = [...elementsNumberOnly].filter( | ||
| (item) => typeof item == "number" | ||
| ); | ||
|
Comment on lines
+2
to
+5
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could these filters be combined so that the code only needs to filter the array once? |
||
|
|
||
| if (elementsPureNumber.length === 0) { | ||
| return -Infinity; | ||
| } else if (elementsPureNumber.length === 1) { | ||
| return elementsPureNumber[0]; | ||
| } else if (elementsPureNumber.length > 1) { | ||
| const elementsSorted = elementsPureNumber.sort((a, b) => a - b); | ||
| const elementsMax = elementsSorted.toSpliced(0, elementsSorted.length - 1); | ||
| return elementsMax[0]; | ||
|
Comment on lines
+12
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| } | ||
|
|
||
| module.exports = findMax; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,21 @@ | ||
| function sum(elements) { | ||
| const elementsNumberOnly = [...elements].filter((element) => !isNaN(element)); | ||
| const elementsPureNumber = [...elementsNumberOnly].filter( | ||
| (item) => typeof item == "number" | ||
| ); | ||
|
Comment on lines
+2
to
+5
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you expect from the following function calls (on extreme cases)? |
||
|
|
||
| if (elementsPureNumber.length === 0) { | ||
| return 0; | ||
| } else if (elementsPureNumber.length === 1) { | ||
| return elementsPureNumber[0]; | ||
| } else if (elementsPureNumber.length > 1) { | ||
| const iterator = elementsPureNumber.values(); | ||
| let sumOfArray = 0; | ||
| for (const value of iterator) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for-of loop works on array directly (arrays are iterable). You could just write |
||
| sumOfArray += value; | ||
| } | ||
| return sumOfArray; | ||
| } | ||
|
Comment on lines
+7
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code on lines 13-17 also works for array of any size. If you prefer having less code, you could consider using the same code to handle all cases. That way, you don't need to use if-else. |
||
| } | ||
|
|
||
| module.exports = sum; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,24 +13,62 @@ 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") | ||
| describe("sum", () => { | ||
| it("returns infinity if input is an empty list", () => { | ||
| expect(sum([])).toBe(0); | ||
| }); | ||
|
|
||
| // Given an array with just one number | ||
| // When passed to the sum function | ||
| // Then it should return that number | ||
| [[3], [1], [2], [50], [100], [1]].forEach((val) => | ||
| it(`returns the same number when array with one number is input (${val})`, () => | ||
| expect(sum(val)).toBe(val[0])) | ||
| ); | ||
|
|
||
| // Given an array containing negative numbers | ||
| // When passed to the sum function | ||
| // Then it should still return the correct total sum | ||
| [ | ||
| { input: [10, 20, 30], expected: 60 }, | ||
| { input: [5, -50], expected: -45 }, | ||
| { input: [-100, 200], expected: 100 }, | ||
| { input: [1, -1, -2, 2], expected: 0 }, | ||
| ].forEach(({ input, expected }) => | ||
| it(`returns the sum when array contains negative or positive numbers [${input}]`, () => | ||
| 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 | ||
| [ | ||
| { input: [1.1, 1.2], expected: 2.3 }, | ||
| { input: [-1.1, -2.4, 1.1], expected: -2.4 }, | ||
| { input: [-15.38, -3.2, -5.01, -10.0], expected: -33.59 }, | ||
| ].forEach(({ input, expected }) => | ||
| it(`returns the sum in array contains decimal number [${input}]`, () => | ||
| expect(sum(input)).toEqual(expected)) | ||
| ); | ||
|
Comment on lines
+46
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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); // falseCan you find a more appropriate way to test a value (that involves decimal number calculations) for equality? Suggestion: Look up
|
||
|
|
||
| // 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: ["hey", 10, "hi", 60, 10], expected: 80 }, | ||
| { input: [9, "orange", 1], expected: 10 }, | ||
| { input: [-9, "3", "apple", 1], expected: -8 }, | ||
| { input: [9, "grape", "banana"], expected: 9 }, | ||
| ].forEach(({ input, expected }) => | ||
| it(`returns the sum in array contains decimal number [${input}]`, () => | ||
| expect(sum(input)).toEqual(expected)) | ||
| ); | ||
|
|
||
| // 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 | ||
| [["hey", "hi"], ["apple"], ["banana", true]].forEach((input) => | ||
| it(`returns 0 when the array contains only non-number elements (${input})`, () => | ||
| expect(sum(input)).toBe(0)) | ||
| ); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.filter()? (This comment applies to all the code that that uses.filter().)