-
-
Notifications
You must be signed in to change notification settings - Fork 268
Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 1 | Data Groups #1032
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
c4b663d
4ee9bf8
1b6cc9b
4d87c5a
074cc7c
1f24b43
2993b07
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,17 @@ | ||
| function dedupe() {} | ||
| // Return a new array with duplicate values removed. | ||
| // Keep the first occurrence of each value. | ||
|
|
||
| function dedupe(elements) { | ||
| const uniqueElements = []; | ||
|
|
||
| for (const element of elements) { | ||
| // Add the element only if it is not already in the result array | ||
| if (!uniqueElements.includes(element)) { | ||
| uniqueElements.push(element); | ||
| } | ||
| } | ||
|
|
||
| return uniqueElements; | ||
| } | ||
|
|
||
| module.exports = dedupe; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,11 @@ const dedupe = require("./dedupe.js"); | |
| /* | ||
| Dedupe Array | ||
|
|
||
| 📖 Dedupe means **deduplicate** | ||
| <<<<<<< HEAD | ||
| 📖 Dedupe means deduplicate | ||
| ======= | ||
| Dedupe means deduplicate | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
|
|
||
| In this kata, you will need to deduplicate the elements of an array | ||
|
|
||
|
|
@@ -16,12 +20,37 @@ 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("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 | ||
| <<<<<<< HEAD | ||
| test("given an array with no duplicates, it returns the same values", () => { | ||
| expect(dedupe([1, 2, 3])).toEqual([1, 2, 3]); | ||
| ======= | ||
| test("given an array with no duplicates, it returns a copy of the original array", () => { | ||
| const input = [1, 2, 3]; | ||
| const result = dedupe(input); | ||
|
|
||
| expect(result).toEqual(input); | ||
| expect(result).not.toBe(input); | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
| }); | ||
|
Comment on lines
27
to
+41
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. 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. 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 duplicate strings, removes duplicates and preserves first occurrence", () => { | ||
| expect(dedupe(["a", "a", "a", "b", "b", "c"])).toEqual(["a", "b", "c"]); | ||
| }); | ||
|
|
||
| test("given an array with duplicate numbers, removes duplicates and preserves first occurrence", () => { | ||
| expect(dedupe([5, 1, 1, 2, 3, 2, 5, 8])).toEqual([5, 1, 2, 3, 8]); | ||
| }); | ||
|
|
||
| test("given a mixed duplicate order, removes duplicates and preserves first occurrence", () => { | ||
| expect(dedupe([1, 2, 1])).toEqual([1, 2]); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,24 @@ | ||
| // Find the largest numerical value in an array. | ||
| // Non-number values should be ignored. | ||
|
|
||
| function findMax(elements) { | ||
| let maxValue = -Infinity; | ||
|
|
||
| for (const element of elements) { | ||
| <<<<<<< HEAD | ||
| // Only compare values that are real numbers | ||
| if (typeof element === "number" && !Number.isNaN(element)) { | ||
| if (element > maxValue) { | ||
| maxValue = element; | ||
| } | ||
| ======= | ||
| if (Number.isFinite(element) && element > maxValue) { | ||
| maxValue = element; | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
| } | ||
| } | ||
|
|
||
| return maxValue; | ||
| } | ||
|
|
||
| module.exports = findMax; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,29 +15,52 @@ 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, returns that number", () => { | ||
| expect(findMax([42])).toBe(42); | ||
| }); | ||
|
|
||
| // 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 an array with positive and negative numbers, returns the largest number", () => { | ||
| expect(findMax([-10, 3, 25, -1])).toBe(25); | ||
| }); | ||
|
|
||
| // 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 only negative numbers, returns the largest one", () => { | ||
| expect(findMax([-9, -2, -15, -4])).toBe(-2); | ||
| }); | ||
|
|
||
| // 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, returns the largest decimal", () => { | ||
| expect(findMax([1.2, 3.8, 2.4])).toBe(3.8); | ||
| }); | ||
|
|
||
| // 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, ignores them and returns the max", () => { | ||
| <<<<<<< HEAD | ||
| expect(findMax(["hey", 10, "hi", 60, 10])).toBe(60); | ||
| ======= | ||
| expect(findMax(["hey", 10, "300", "hi", 60, 10])).toBe(60); | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
| }); | ||
|
Comment on lines
+53
to
+59
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. When a string representing a valid numeric literal (for example, To test if the function can correctly ignore non-numeric values, you could include a string such as |
||
|
|
||
| // 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, returns -Infinity", () => { | ||
| expect(findMax(["apple", null, undefined, "banana"])).toBe(-Infinity); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,21 @@ | ||
| // Sum the numerical values in an array. | ||
| // Non-number values should be ignored. | ||
|
|
||
| function sum(elements) { | ||
| let total = 0; | ||
|
|
||
| for (const element of elements) { | ||
| <<<<<<< HEAD | ||
| // Only add values that are real numbers | ||
| if (typeof element === "number" && !Number.isNaN(element)) { | ||
| ======= | ||
| if (Number.isFinite(element)) { | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
| total += element; | ||
| } | ||
| } | ||
|
|
||
| return total; | ||
| } | ||
|
|
||
| module.exports = sum; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,24 +13,45 @@ 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 an array with negative numbers, returns the correct sum", () => { | ||
| expect(sum([-5, 10, -2])).toBe(3); | ||
| }); | ||
|
|
||
| // 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 numbers, returns the correct sum", () => { | ||
| <<<<<<< HEAD | ||
| expect(sum([1.5, 2.5, 3])).toBe(7); | ||
| ======= | ||
| expect(sum([1.2, 0.6, 0.005])).toBeCloseTo(1.805, 10); | ||
| >>>>>>> a22ed15 (Address mentor feedback for sprint 1 data groups) | ||
| }); | ||
|
Comment on lines
+37
to
+43
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
Author
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. Hi @cjyuan, Thank you for the feedback. I have updated the code and tests based on your comments:
I re-ran all Sprint 1 test suites and all tests are passing.
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. All these changes sounds good, but your actions somehow introduced merge conflicts to the files. Can you resolve the conflicts? |
||
|
|
||
| // 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 with non-number values, ignores them and sums only numbers", () => { | ||
| 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 an array with only non-number values, returns 0", () => { | ||
| expect(sum(["apple", null, undefined, "banana"])).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.
This is a unresolved merge conflict. You need to delete the "merge conflict markers" and the code that you do not want to keep.
There are several others on the files in this branch.
Can you fix them?