Skip to content

Manchester | 26-ITP-May | Joanne O'Malley | Sprint 2 | Exercises#1247

Open
joanne342 wants to merge 21 commits into
CodeYourFuture:mainfrom
joanne342:sprint2
Open

Manchester | 26-ITP-May | Joanne O'Malley | Sprint 2 | Exercises#1247
joanne342 wants to merge 21 commits into
CodeYourFuture:mainfrom
joanne342:sprint2

Conversation

@joanne342

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

Changelist

Sprint 2 coursework

@joanne342 joanne342 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 14, 2026
@LonMcGregor LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jul 20, 2026

@LonMcGregor LonMcGregor 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.

Good start on this task, I've left some comments for you to answer

Comment thread Sprint-2/implement/contains.js Outdated
@@ -1,3 +1,5 @@
function contains() {}
function contains(obj, property_name) {
return property_name in obj;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this consider the difference between defined properties and built-in properties?

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.

Hello

I changed

return property_name in obj;

to

return Object.prototype.hasOwnProperty.call(obj, property_name);

and added the test case

test("returns false for inherited properties like toString", () => {
expect(contains({}, "toString")).toBe(false);
});

to illustrate the difference.

const splitByAnds = queryString.split("&");

const splitByEquals = splitByAnds.map(x => {
const match = x.match(/^([^=]*)(?:=(.*))?$/);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you explain what this regular expression is doing?

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.

([^=]*)

Make a capturing group of zero or more characters that are not =, grabbing as many as possible until the first equals sign (if one exists).

===

(?:=(.*))?

The second capturing group (.*) grabs everything after the first equals sign, including any additional equals signs. It is wrapped in a non-capturing group (?:...) because we don't want the = part itself to create another capture group.

===

It functions similarly to .split("=") with a limit of one split. A normal .split("=") does not work here because query values can themselves contain equals signs. For example, equation=a=b-2 should become:

["equation", "a=b-2"]

rather than:

["equation", "a", "b-2"]

In Python this is equivalent to:

"equation=a=b-2".split("=", 1)


const counts = {};
for (const item of uniqueItems) {
counts[item] = items.filter(x => x === item).length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Imagine I had the input of ['a', 'a', 'a', ...] - How many times will .filter run? Is that an optimal solution?

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.

The current solution loops through the array once to build the unique list, then runs .filter() for each unique item.

If the array contains only duplicates like ['a', 'a', 'a'], the set reduces it to ['a'], so .filter() only runs once. This gives us O(n).

If every item is unique, .filter() runs once for every item in the array, and each filter scans the whole array again. This gives us O(n²).

A more efficient approach is to loop through the array once and store the counts as we go using an object or dictionary.

@LonMcGregor LonMcGregor 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. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 20, 2026
@joanne342 joanne342 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants