Skip to content

West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 2 | Sprint 2 Exercises - #1433

Open
maryam-devio wants to merge 1 commit into
CodeYourFuture:mainfrom
maryam-devio:dataGroup-Sprint2
Open

West-Midlands | 26-ITP-May | Maryam Janjua | Sprint 2 | Sprint 2 Exercises#1433
maryam-devio wants to merge 1 commit into
CodeYourFuture:mainfrom
maryam-devio:dataGroup-Sprint2

Conversation

@maryam-devio

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

I attempted all the exercises according to the requirements. I debugged the code, completed the tasks, and tested my solutions to make sure they work as expected.

@maryam-devio maryam-devio 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. Module-Data-Groups The name of the module. labels Aug 14, 2026
Comment thread Sprint-2/debug/recipe.js
ingredients:
${recipe}`);
console.log(`${recipe.title} serves ${recipe.serves}`);
for(let values of Object.values(recipe.ingredients)){

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.

  • Is it necessary to use Object.values() on line 18?

  • If we do not need to reassign a value to the loop variable, common practice is to declare it using const.

Comment on lines +5 to +12
let getKey = Object.keys(obj);
for (let element of getKey){
if(element === item){
return true
}
}
return false;
}

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.

This works.

Do check out Object.hasOwn() and also
use AI to find out the trade-off among different ways to check if an object contains a particular key.

Comment on lines +45 to +47
test("return false if it's not an object", () => {
expect(contains([], 6)).toBe(false);
}); No newline at end of file

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.

When a function does not test if the first argument is an array, contains([], 6) could also return false simply because 6 is not a key of the empty array.

A proper test should use a non-empty array along with a valid
key to ensure the function returns false specifically because the first argument is an array, not because the key is missing.

key = decodeURIComponent(key);
value = decodeURIComponent(value);

if (Object.prototype.hasOwnProperty.call(queryParams, key)) {

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.

Could also use Object.hasOwn().

Comment on lines +2 to +10
const result = arr.reduce((obj, item) => {
if (obj [item]) {
obj[item] = obj[item] + 1;
}
else{
obj[item] = 1;
}
return obj;
}, {});

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()

@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 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Data-Groups The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants