Skip to content

Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 2 | Data Groups#1042

Open
Richiealx wants to merge 13 commits intoCodeYourFuture:mainfrom
Richiealx:coursework/sprint-2-data-groups
Open

Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 2 | Data Groups#1042
Richiealx wants to merge 13 commits intoCodeYourFuture:mainfrom
Richiealx:coursework/sprint-2-data-groups

Conversation

@Richiealx
Copy link

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 pull request contains my completed work for Sprint 2 - Data Groups.

Debug

  • Fixed address.js (correct object property access)
  • Fixed author.js (correct object iteration)
  • Fixed recipe.js (proper formatting of output)

Implement

  • Implemented contains using own-property checking (avoids inherited properties)
  • Implemented createLookup to convert key-value pairs into an object
  • Implemented tally using Object.create(null) to handle edge cases like "toString"
  • Improved parseQueryString to:
    • handle values containing "="
    • handle missing values and keys
    • ignore trailing "&"
    • decode URL-encoded keys and values

Interpret

  • Analysed and explained the issue in invert.js
  • Fixed implementation using bracket notation
  • Added tests to verify correctness

Stretch

  • Implemented countWords with:
    • punctuation handling
    • case normalisation
    • whitespace handling
    • safe object creation to handle words like "constructor"
  • Refactored calculateMode into smaller helper functions:
    • frequency tracking
    • mode selection

Testing

  • All tests pass successfully
  • 7 test suites and 27 tests passing

@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 18, 2026
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Code looks good. I just have some suggestions.

Comment on lines +19 to +21
for (const ingredient of recipe.ingredients) {
console.log(ingredient);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since ingredient values are separated by '\n' in the output, we could also use Array.prototype.join() to construct the equivalent string and then output the resulting string.

}

// Check if the object has the property as its own key
return Object.prototype.hasOwnProperty.call(obj, propertyName);
Copy link
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 +18 to +21
// Validate propertyName
if (typeof propertyName !== "string" || propertyName.length === 0) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Values of non-string type can be used as property names; they are just implicitly converted to equivalent strings. For example,
let obj = {};
obj[3.0] = 12;           // 3.0 is treated as "3" when used as key
console.log(obj["3"]);   // output 12 
console.log(obj[4-1]);   // output 12 too
console.log(Object.hasOwn(obj, 3));  // output true
  • Empty string can also serve as key.

Comment on lines 37 to 41
// Extract the key and everything after the first "=" as the value
const key = pair.slice(0, separatorIndex);
const value = pair.slice(separatorIndex + 1);

queryParams[key] = value;
Copy link
Contributor

Choose a reason for hiding this comment

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

No change required.

Note:

  • In real query string, both key and value are percent-encoded or URL encoded in the URL.
    For example, the string "5%" is encoded as "5%25". So to get the actual value of "5%25"
    (whether it is a key or value in the query string), we need to call a function to decode it.

  • You can also explore the URLSearchParams API.

Comment on lines +17 to +30
const counts = {};

// Loop through each item in the array
for (const item of items) {
// If the item already exists in the object, increase the count
if (counts[item]) {
counts[item] += 1;
} else {
// Otherwise initialise it
counts[item] = 1;
}
}

return counts;
Copy link
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.

Copy link
Author

@Richiealx Richiealx Mar 23, 2026

Choose a reason for hiding this comment

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

Hi @cjyuan,

Thank you for your feedback.

I’ve now updated my implementation to address all the points raised:

  • Updated contains to use proper own-property checking and improved the tests to cover inherited properties, arrays, non-string keys, and edge cases
  • Fixed tally by using an object with no inherited properties and added an edge-case test for values like "toString"
  • Improved querystring to handle URL decoding and additional edge cases
  • Replaced the remaining test.todo in lookup.test.js with full test coverage, including validation and malformed input cases
  • Reviewed recipe.js and applied the suggested improvement using join for cleaner output

All test suites are now passing with no remaining TODO tests.
I also corrected the branch issue so this PR now only contains Sprint-2 files.

Thanks again for the helpful suggestions.

@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 Mar 21, 2026
@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 23, 2026
@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 Mar 23, 2026
@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 23, 2026
@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 Mar 23, 2026
@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 23, 2026
@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 Mar 23, 2026
@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 23, 2026
@cjyuan
Copy link
Contributor

cjyuan commented Mar 23, 2026

On this PR branch, I don't see any changes made to the files in the Sprint-2 folder (since my previous review).

That is, I don't see some of the changes which you said you did:

  • Updated contains to use proper own-property checking and improved the tests to cover inherited properties, arrays, non-string keys, and edge cases
  • Fixed tally by using an object with no inherited properties and added an edge-case test for values like "toString"
  • Improved querystring to handle URL decoding and additional edge cases
  • Reviewed recipe.js and applied the suggested improvement using join for cleaner output

@cjyuan cjyuan removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 23, 2026
@Richiealx
Copy link
Author

Thank you for pointing that out.

You were right. Those specific changes were not yet present on the Sprint-2 PR branch. I’ve now updated the Sprint-2 files to include the requested fixes:

  • updated contains to use proper own-property checking and improved the tests to cover inherited properties, arrays, non-string keys, and empty-string keys
  • fixed tally to use an object with no inherited properties and added the "toString" edge-case test
  • improved querystring to handle URL decoding and added an extra decoding test
  • updated recipe.js to use join() for cleaner ingredient output

I also fixed countWords so the full Sprint-2 test suite passes cleanly.

All Sprint-2 tests are now passing on this PR branch:

  • 7 test suites passed
  • 36 tests passed

@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 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. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants