-
-
Notifications
You must be signed in to change notification settings - Fork 270
Sheffield | 26-ITP-Jan | Karim M'hamdi | Sprint 1 | Module-Data-Groups #1029
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
7d07f44
ae69f89
1cb5870
f8bcc92
396a9c0
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 |
|---|---|---|
| @@ -1 +1,13 @@ | ||
| function dedupe() {} | ||
| function dedupe(array) { | ||
| let seen = new Set(); | ||
| let result = []; | ||
| for (let item of array) { | ||
| if (!seen.has(item)) { | ||
| seen.add(item); | ||
| result.push(item); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| module.exports = dedupe; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,22 @@ | ||
| function findMax(elements) { | ||
| let max = -Infinity; | ||
| let foundNumber = false; | ||
|
|
||
| for (let i = 0; i < elements.length; i++) { | ||
| if (Number.isFinite(elements[i])) { | ||
| foundNumber = true; | ||
|
|
||
| if (elements[i] > max) { | ||
| max = elements[i]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!foundNumber) { | ||
| return null; | ||
| } | ||
|
|
||
| return max; | ||
| } | ||
|
|
||
| module.exports = findMax; | ||
| module.exports = findMax; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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("given an empty array, returns null", () => { | ||
| expect(findMax([])).toBe(null); | ||
| }); | ||
|
|
||
| // 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 , returns that number", () => { | ||
| expect(findMax([7])).toBe(7); | ||
| }); | ||
|
|
||
| // 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, returns the largest number", () => { | ||
| expect(findMax([-10, 0, 5, -3, 8])).toBe(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 only negative numbers, returns the largest negative number", () => { | ||
| expect(findMax([-10, -4, -2, -30])).toBe(-2); | ||
| }); | ||
|
|
||
| // Given an array with decimal numbers | ||
| // When passed to the max function | ||
| // Then it should return the largest decimal number | ||
| test("given decimal numbers, returns the largest decimal number", () => { | ||
| expect(findMax([1.2, 3.7, 2.5])).toBe(3.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 and returns the max", () => { | ||
| 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 | ||
| test("given only non-number values, returns null", () => { | ||
| expect(findMax(["hey", "hi", null, undefined, NaN])).toBe(null); | ||
| }); | ||
|
Comment on lines
+61
to
+63
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 test on line 62 make it looks like the function considers You can also consider treating an array with only non-number values the same as an array with no numbers, and give all arrays containing no numbers the same treatment? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,11 @@ | ||
| function sum(elements) { | ||
| let sum = 0; | ||
| for (let i = 0; i < elements.length; i++) { | ||
| if (Number.isFinite(elements[i])) { | ||
| sum += elements[i]; | ||
| } | ||
| } | ||
| return sum; | ||
| } | ||
|
|
||
| module.exports = sum; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,24 +13,43 @@ 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([])).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, returns that number", () => { | ||
| expect(sum([7])).toBe(7); | ||
| }); | ||
|
|
||
| // 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, returns the correct total", () => { | ||
| expect(sum([-10, -3, -2])).toBe(-15); | ||
| }); | ||
|
|
||
| // 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, returns the correct total", () => { | ||
| expect(sum([1.5, 2.5, 3])).toBe(7); | ||
| }); | ||
|
Comment on lines
+37
to
+39
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 | ||
| test("ignores non-number values and returns the sum", () => { | ||
| 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 | ||
| test("given only non-number values, returns 0", () => { | ||
| expect(sum(["hey", "hi", null, undefined, NaN])).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.
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
60 < "300"evaluates totrue.To test if the function can correctly ignore non-numeric values,
consider including a string such as
"300"in the relevant test cases.