Skip to content

Glasgow | 26-Jul-SDC | Shreef Ibrahim | Sprint 2 | Analyse Refactor Functions - #239

Open
shreefAhmedM wants to merge 5 commits into
CodeYourFuture:mainfrom
shreefAhmedM:analyseRefactorFunctions
Open

shreefAhmedM wants to merge 5 commits into
CodeYourFuture:mainfrom
shreefAhmedM:analyseRefactorFunctions

Conversation

@shreefAhmedM

@shreefAhmedM shreefAhmedM commented Oct 12, 2026 •

Copy link
Copy Markdown

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

Task code

CYF-1173

Changelist

This PR analyses and refactors functions to improve efficiency, reduce time complexity, and follow best practices. It documents the original and optimised time and space complexities for each function.

@github-actions

This comment has been minimized.

@shreefAhmedM shreefAhmedM added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 12, 2026
@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 12, 2026
@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@shreefAhmedM shreefAhmedM changed the title analyse and refactor sprint functions Glasgow | 26-Jul-SDC | Shreef Ibrahim | Sprint 1 |analyseRefactorFunctions Oct 12, 2026
@shreefAhmedM shreefAhmedM added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 13, 2026

@illicitonion illicitonion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This generally looks good, but has a lot of lack of attention to detail - please take a look and fix up the comments

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You haven't explained the complexity of the before or after code

@illicitonion illicitonion 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 Oct 13, 2026
@shreefAhmedM
shreefAhmedM deleted the analyseRefactorFunctions branch October 14, 2026 14:24
@shreefAhmedM
shreefAhmedM restored the analyseRefactorFunctions branch October 14, 2026 14:34
@shreefAhmedM shreefAhmedM reopened this Oct 14, 2026
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@shreefAhmedM shreefAhmedM changed the title Glasgow | 26-Jul-SDC | Shreef Ibrahim | Sprint 1 |analyseRefactorFunctions Glasgow | 26-Jul-SDC | Shreef Ibrahim | Sprint 2 | Analyse Refactor Functions Oct 16, 2026
@github-actions

This comment has been minimized.

@shreefAhmedM shreefAhmedM added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 17, 2026

@cjyuan cjyuan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Changes looks good.

return uniqueValues;
};

console.log(findCommonItems([2, 5, 7], [5, 6, 7]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code submitted to a PR should be free of debugging code.

Comment on lines +13 to +17
const allowedItems = new Set(secondArray);

const uniqueValues = [
...new Set(firstArray.filter((item) => allowedItems.has(item))),
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note:
Most Set implementations (regardless of languages) support the common set operations such as union, intersection, difference, subset, superset, equality, add, remove, contains, size, clear, and is empty.
You could consider taking advantage of them for optimal performance.

@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 Nov 10, 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.

3 participants