-
-
Notifications
You must be signed in to change notification settings - Fork 208
Glasgow | 25-ITP-SEP | Hanna Mykytiuk | Sprint 1 | Coursework #817
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
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 |
|---|---|---|
| @@ -1 +1,10 @@ | ||
| function dedupe() {} | ||
| function dedupe(array) { | ||
| if (array.length == 0){ // check if array is empty | ||
| return []; | ||
| } | ||
| let mySet = new Set(array); // create set mySet | ||
| let dedupeArray = Array.from (mySet); // create new array from the set mySet | ||
| return dedupeArray; | ||
| } | ||
|
|
||
| module.exports = dedupe; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,29 @@ 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", () => { | ||
| const currentOutput = dedupe([]); | ||
| const targetOutput = []; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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 should return a copy of the original array", () => { | ||
| const currentOutput = dedupe([5, 1, 2, 3, 8]); | ||
| const targetOutput = [5, 1, 2, 3, 8]; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
Comment on lines
27
to
+32
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 strings or number, it should remove the duplicate values, preserving the first occurence of each element", () => { | ||
| const currentOutput = dedupe([5, "1", "1", 5, 8]); | ||
| const targetOutput = [5, "1", 8]; | ||
|
|
||
| console.log(currentOutput); | ||
| console.log(targetOutput); | ||
|
|
||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,19 @@ | ||
| function findMax(elements) { | ||
| //filter array for numbersOnly | ||
| const numbersOnly = elements.filter(item => !isNaN(item)); | ||
| if (numbersOnly.length == 0){ // check if array is empty | ||
| return -Infinity; | ||
| } | ||
| if (numbersOnly.length === 1){ //check if they has one element | ||
| return numbersOnly[0]; | ||
| } | ||
| let max = numbersOnly[0];// find a max number | ||
| for (let i=1; i<numbersOnly.length; i++){ | ||
| if (numbersOnly[i]> max){ | ||
| max = numbersOnly[i]; | ||
| } | ||
| } | ||
| return max; | ||
| } | ||
|
|
||
| module.exports = findMax; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,28 +16,63 @@ 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, it should return -Infinity", () => { | ||
| const currentOutput = findMax([]); | ||
| const targetOutput = -Infinity; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should return that number", () => { | ||
| const currentOutput = findMax([1]); | ||
| const targetOutput = 1; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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 both positive and negative numbers it should return the largest number overall", () => { | ||
| const currentOutput = findMax([-2, 2, 3, -3]); | ||
| const targetOutput = 3; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should returnthe closest one to zero", () => { | ||
| const currentOutput = findMax([-1, -2, -3, -4]); | ||
| const targetOutput = -1; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should return the largest decimal number", () => { | ||
| const currentOutput = findMax([0.2, 0.45, 5.3, 1.5]); | ||
| const targetOutput = 5.3; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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 currentOutput = findMax(["a", "!", 3, 5, "0"]); | ||
| const targetOutput = 5; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
Comment on lines
+65
to
+69
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. Does the following function call returned the value you expect? findMax(["1000", 1]);
findMax([1000, "1"]); |
||
|
|
||
| // 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", () => { | ||
| const currentOutput = findMax(["2", "3", "$", "%"]); | ||
| const targetOutput = "3"; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
Comment on lines
+74
to
+78
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 is "3" considered the least surprising value? What's your rationale? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,21 @@ | ||
| function sum(elements) { | ||
| if (elements.length == 0){ // check if array is empty | ||
| return 0; | ||
| } | ||
| if (elements.length === 1){ //check if array has one element | ||
| return elements[0]; | ||
| } | ||
| let sum = 0; | ||
| let isNumbersPresent = elements.some(item => typeof item === 'number' && !isNaN(item)); | ||
| for (let i =0; i<elements.length; i++){ //sum of elements | ||
| if (typeof elements[i] === 'number'){ //check if element is number | ||
| sum += elements[i]; | ||
| } | ||
| if (isNumbersPresent == false && !isNaN(elements[i])){ //1) check if array doesnt have numbers; 2)check if element can be conversted to number | ||
| sum += parseFloat(elements[i]); // add converted to num element | ||
| } | ||
| } | ||
| return sum | ||
|
Comment on lines
+2
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. Does the following function calls return the value you expected? |
||
| } | ||
|
|
||
| module.exports = sum; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,31 +6,61 @@ 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"); | ||
| 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, it should return 0", () => { | ||
| const currentOutput = sum ([]); | ||
| const targetOutput = 0; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // Given an array with just one number | ||
| // When passed to the sum function | ||
| // Then it should return that number | ||
| test("given an array with just one number, it should return that number", () => { | ||
| const currentOutput = sum ([1]); | ||
| const targetOutput = 1; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should still return the correct total sum", () => { | ||
| const currentOutput = sum ([-2, -3, -4, -5]); | ||
| const targetOutput = -14; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should return the correct total sum", () => { | ||
| const currentOutput = sum ([2.5, 3.1, 4.9]); | ||
| const targetOutput = 10.5; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
Comment on lines
+44
to
+48
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("given an array containing non-number values, it should return the sum of the numerical elements", () => { | ||
| const currentOutput = sum (["a", "!", 3, 5, ","]); | ||
| const targetOutput = 8; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
|
|
||
| // 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, it should return the least surprising value given how it behaves for all other inputs", () => { | ||
| const currentOutput = sum (["a", "!", "3", "5", ","]); | ||
| const targetOutput = 8; | ||
| expect(currentOutput).toEqual(targetOutput); | ||
| }); | ||
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.
numbersOnlyis 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 and
write the code on line 16 as
Note:
sortedis a more meaningful name so it is good to keep it.