Skip to content

London | May-2025 | Halyna Kozlovska | Module Data Groups | Sprint 1 #542

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
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

halyna-k
Copy link

@halyna-k halyna-k commented Jul 1, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

  • Implemented findMax, sum, dedupe, and includes functions.
  • Added tests for various scenarios, including empty arrays, duplicates, mixed types, decimals, and null values.
  • Refactored includes to use for...of for cleaner code.
  • Ensured all functions handle edge cases and pass tests.

Questions

Ask any questions you have for your reviewer.

halyna-k added 6 commits June 28, 2025 15:53
- fix and improve median calculation logic
- refactor findMax function;
- add comprehensive tests covering all cases
- refactor max.test.js to use a single describe block
- refactor sum function;
- add comprehensive tests for all cases:
empty arrays, single numbers, negative numbers, decimals, mixed types, and arrays with only non-numeric values.
- implement dedupe function;
- add tests  to verify the function's behavior for various cases: empty arrays, arrays with no duplicates, array with string or number duplicates
- refactor includes function to use for...of loop;
- update tests assertions to use toBe instead of toEqual;
- fixed the empty array test to include a target argument
@halyna-k halyna-k added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Participant to add when requesting review labels Jul 1, 2025
@halyna-k halyna-k linked an issue Jul 2, 2025 that may be closed by this pull request
@AdnanGondal AdnanGondal self-assigned this Jul 14, 2025
@AdnanGondal AdnanGondal added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Participant to add when requesting review labels Jul 14, 2025
@@ -1 +1,10 @@
function dedupe() {}
function dedupe(array) {
let result = [];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For very large arrays, a Set can provide better performance than using an Array. Consider updating this implementation to use a set. See here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdnanGondal Thanks for your comment — good point! Using a Set makes lookups faster (O(1) vs O(n)) and keeps values unique, which is great for large arrays. Here the time and space difference is small because the dataset is tiny, but I’ll keep Set in mind for future cases where performance matters more.

sum += element;
}
}
return Math.round(sum * 100) / 100;;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me what this line does?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdnanGondal Line 9 rounds the total sum to two decimal places before returning it.

Copy link

@AdnanGondal AdnanGondal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great attempt - have just added a comment and question, please address those and I will Complete

@AdnanGondal AdnanGondal added Reviewed Volunteer to add when completing a review and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 14, 2025
@halyna-k halyna-k requested a review from AdnanGondal July 14, 2025 20:04
@halyna-k halyna-k added the Needs Review Participant to add when requesting review label Jul 14, 2025
Copy link

@AdnanGondal AdnanGondal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@AdnanGondal AdnanGondal added Complete Volunteer to add when work is complete and review comments have been addressed and removed Needs Review Participant to add when requesting review Reviewed Volunteer to add when completing a review labels Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and review comments have been addressed 📅 Sprint 1 Assigned during Sprint 1 of this module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TECH ED] Complete sprint 1 exercises
2 participants