-
-
Notifications
You must be signed in to change notification settings - Fork 280
Cape Town | 26-ITP-Jan | Pretty Taruvinga | Sprint 1 | Data Groups #1104
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
Open
Pretty548
wants to merge
23
commits into
CodeYourFuture:main
Choose a base branch
from
Pretty548:coursework/Sprint-1
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
53fa90f
implement correct median calculation
86132f9
Co-authored-by: Isaac Abodunrin <bytesandroses@users.noreply.github.c…
33ec99a
implement dedupe function to remove duplicates from array
4bf6702
git commit -m implement dedupe function and add comprehensive tests"
5db3aac
git commit -m implement max, sum, and dedupe functions with passing …
fc16e68
git commit -m implement findMax with support for mixed data and edge …
0e7b9da
implemented sum function with support for mixed data
d4e734a
resolve precision bug in sum
459eb79
update includes to use for...of loop
0a5f5cc
fixed a wrong spelling
d55358c
Merge branch 'main' into coursework/Sprint-1
Pretty548 5514777
Co-authored-by: Isaac Abodunrin <bytesandroses@users.noreply.github.c…
eb1b8f2
Ensure dedupe returns a new array reference
d0ec9a3
Return -Infinity for empty array to match spec
90c1ff9
Fix expected values and remove duplicate tests
6aa82cf
Ignore non-number values when calculating maximum
afa2fdb
Remove unnecessary precision reduction
43cf84a
Improve dedupe tests by adding duplicate cases and ensuring correct b…
a548d71
Strengthen dedupe test by adding length and content checks to prevent…
ad9af40
Update dedupe test to use fixed expected values and check input immut…
ad01a87
Fix dedupe test to prevent false positives and check input immutability
c1b088d
Improve dedupe test to check immutability and order
739b799
Co-authored-by: Isaac Abodunrin <bytesandroses@users.noreply.github.c…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,7 @@ | ||
| function dedupe() {} | ||
| function dedupe(list) { | ||
| if (!Array.isArray(list)) { | ||
| return null; | ||
| } | ||
| return list.filter((value, index, self) => self.indexOf(value) === index); | ||
| } | ||
| module.exports = dedupe; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,16 +13,35 @@ E.g. dedupe([1, 2, 1]) returns [1, 2] | |
|
|
||
| // Acceptance Criteria: | ||
|
|
||
| // 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"); | ||
|
|
||
| // Given an array with no duplicates | ||
| // When passed to the dedupe function | ||
| // Then it should return a copy of the original array | ||
|
|
||
| // 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. | ||
| test("Given an empty array, when passed to the dedupe function, then it should return an empty array", () => { | ||
| expect(dedupe([])).toEqual([]); | ||
| }); | ||
|
|
||
| test("Given a non-array value, when passed to the dedupe function, then it should return null", () => { | ||
| expect(dedupe("not an array")).toBeNull(); | ||
| expect(dedupe(123)).toBeNull(); | ||
| expect(dedupe({})).toBeNull(); | ||
| }); | ||
|
|
||
| test("Given an array with no duplicates, when passed to the dedupe function, then it should return a copy of the original array", () => { | ||
| const input = ["a", "b", "c", "d"]; | ||
| const copyOfInput = ["a", "b", "c", "d"]; | ||
|
|
||
| const result = dedupe(input); | ||
|
|
||
| expect(result).toEqual(copyOfInput); | ||
| expect(result).not.toBe(input); | ||
| expect(input).toEqual(copyOfInput); | ||
| }); | ||
|
Comment on lines
+26
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. When both |
||
|
|
||
| test("Given an array with strings or numbers, when passed to the dedupe function, then it should remove the duplicate values, preserving the first occurrence of each element", () => { | ||
| expect(dedupe(["a", "a", "b", "c", "c"])).toEqual(["a", "b", "c"]); | ||
| expect(dedupe([1, 2, 2, 3, 4, 4])).toEqual([1, 2, 3, 4]); | ||
| }); | ||
|
|
||
| test("preserves the original order of first occurrences", () => { | ||
| const input = ["b", "a", "b", "c"]; | ||
| const result = dedupe(input); | ||
|
|
||
| expect(result).toEqual(["b", "a", "c"]); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,12 @@ | ||
| function findMax(elements) { | ||
| function max(numbers) { | ||
| let max = -Infinity; | ||
|
|
||
| for (let num of numbers) { | ||
| if (typeof num === "number" && num > max) { | ||
| max = num; | ||
| } | ||
| } | ||
| return max; | ||
| } | ||
|
|
||
| module.exports = findMax; | ||
| module.exports = max; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,14 @@ | ||
| function sum(elements) { | ||
| if (!Array.isArray(elements)) { | ||
| return null; | ||
| } | ||
| let total = 0; | ||
| for (let i = 0; i < elements.length; i++) { | ||
| if (typeof elements[i] === "number" && !isNaN(elements[i])) { | ||
| total += elements[i]; | ||
| } | ||
| } | ||
| return total; | ||
| } | ||
|
|
||
| module.exports = sum; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There is a chance that, even though
resulthas incorrect elements (for example,[]),the two tests could still pass. Can you figure out why, and then fix the tests accordingly?
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.
Thanks for pointing this out!
You're right — the current test isn't strict enough. It only checks that result is equal to input and that it's a different reference, but it doesn't explicitly verify the contents or length of the array. This means a faulty implementation could potentially pass the test under certain conditions.
To fix this, I strengthened the test by adding checks for the array length and ensuring the expected elements are present. This makes the test more robust and prevents false positives.
Let me know if you'd like me to add more edge case tests as well!
Uh oh!
There was an error while loading. Please reload this page.
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.
What if both
resultandinputbecome["c", "b", "a"]?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.
Good point if both input and result were mutated to the same incorrect value (e.g. reversed), the test could still pass.
I’ve updated the test to compare against a fixed expected array and added an assertion to ensure the original input is not mutated. This makes the test stricter and ensures the correct order is preserved.
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.
If you have made any changes, you need to push them to GitHub. Currently, I don't see any new changes since yesterday.