Skip to content

Birmingham | 26-ITP-May | Gabriel Pawuoi | Sprint 2 |debug address/author/recipe, implement contains/lookup/tall - #1410

Open
KhotKeys wants to merge 2 commits into
CodeYourFuture:mainfrom
KhotKeys:sprint-2-solutions
Open

Birmingham | 26-ITP-May | Gabriel Pawuoi | Sprint 2 |debug address/author/recipe, implement contains/lookup/tall#1410
KhotKeys wants to merge 2 commits into
CodeYourFuture:mainfrom
KhotKeys:sprint-2-solutions

Conversation

@KhotKeys

@KhotKeys KhotKeys commented Aug 8, 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

Changelist

  • This PR combines all Sprint-2 solutions into one branch: debugged address/author/recipe, implemented contains/lookup/tally/querystring, fixed invert, refactored mode into smaller functions, and implemented count-words and till.

@KhotKeys KhotKeys added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 8, 2026

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Just one question.

Comment thread Sprint-2/implement/contains.js Outdated
function contains() {}
function contains(obj, prop) {
if (typeof obj !== "object" || obj === null || Array.isArray(obj)) return false;
return Object.prototype.hasOwnProperty.call(obj, prop);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: Could also use Object.hasOwn() -- shorter syntax.

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, I've updated the code to use Object.hasOwn(obj, prop).

Comment thread Sprint-2/implement/tally.js Outdated
Comment on lines +3 to +6
return arr.reduce((acc, item) => {
acc[item] = (acc[item] || 0) + 1;
return acc;
}, {});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the following function call returns the value you expect?

tally(["toString", "toString"]);

Suggestion:

  • Look up an approach to create an empty object with no inherited properties, or
  • use Object.hasOwn()

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.

Great catch on the prototype lookup bug, actually, Passing "toString" breaks standard objects because they inherit from Object.prototype. I have fixed this by initializing the accumulator with Object.create(null) so it has no inherited properties.

@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 Aug 10, 2026
@KhotKeys
KhotKeys force-pushed the sprint-2-solutions branch from 5eabbf1 to 47367c6 Compare August 10, 2026 19:35
@KhotKeys KhotKeys 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 Aug 15, 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. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants