Skip to content

Glasgow | Sheetal Kharab | Module-Complexity | Sprint 1 | Analyse and refactor#32

Open
sheetalkharab wants to merge 5 commits intoCodeYourFuture:mainfrom
sheetalkharab:remove-duplicates-refactor
Open

Glasgow | Sheetal Kharab | Module-Complexity | Sprint 1 | Analyse and refactor#32
sheetalkharab wants to merge 5 commits intoCodeYourFuture:mainfrom
sheetalkharab:remove-duplicates-refactor

Conversation

@sheetalkharab
Copy link
Copy Markdown

@sheetalkharab sheetalkharab commented Oct 14, 2025

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

refactor code

Questions

No

@sheetalkharab sheetalkharab added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 14, 2025
* Time Complexity:
* Space Complexity:
* Optimal Time Complexity:
* Time Complexity:O(2n) originall have this as using 2 separate loops
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

O(n) is the same as O(2n). We ignore the constant multiplier in big-O notation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for the clarification.
I’ve updated the time complexity to O(n), since constant multipliers are ignored in Big-O notation.

* Time Complexity:
* Space Complexity:
* Optimal Time Complexity:
* Time Complexity:O(n+m) it build set and loop through arrays once
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What's the complexity of Array.include()?

.filter() suggests a "loop", and inside the loop .includes() is called repeatedly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@cjyuan Thank you for the clarification.
The previous implementation using .includes() would have been O(n × m).
After refactoring to use a Set, the time complexity is now O(n + m), and space complexity is O(m).

* Optimal Time Complexity:
* Time Complexity:O(n+m) it build set and loop through arrays once
* Space Complexity: store second array in set
* Optimal Time Complexity:O(m+n)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We could also write O(max(m, n))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 22, 2026
@sheetalkharab sheetalkharab added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 3, 2026
@cjyuan
Copy link
Copy Markdown

cjyuan commented Mar 3, 2026

Code is good. Thanks for all responses.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Mar 3, 2026
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 all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants