London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Sprint-2 exercises #809

i786m · 2025-11-02T01:54:37Z

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

Completed the tasks set out in the debug, implement and interpret folders in sprint 2

…ameters

…various cases

cjyuan · 2025-11-11T23:42:45Z

Sprint-2/debug/recipe.js

 console.log(`${recipe.title} serves ${recipe.serves}
  ingredients:
-${recipe}`);
+${recipe.ingredients}`);


This change does not yet satisfy the requirement written on line 6.

cjyuan · 2025-11-12T00:04:19Z

Sprint-2/implement/lookup.js

+      if (Array.isArray(currency)) {
+        result[country] = currency;
+      }
+      if (currency === null || currency === undefined) {
+        continue;
+      }
+
+      if (country.length && currency.length && typeof country === 'string' && typeof currency === 'string') {
+        result[country] ? result[country] = [result[country], currency]
+                        : result[country] = currency;
+      }


Why check country.length and type of country on line 20 but not on line 13?

Why check result[country] on line 21 but not on line 14?

Amended to include checks, and destructure country and currency

cjyuan · 2025-11-12T00:06:20Z

Sprint-2/implement/querystring.js

+    if(pair.indexOf('=') !== pair.lastIndexOf('=')){
+      const firstEqualIndex = pair.indexOf('=');
+      const key = pair.substring(0, firstEqualIndex);
+      const value = pair.substring(firstEqualIndex + 1);
+      queryParams[key] = value;
+      continue;
+    } else {
+      const [key, value] = pair.split("=");
+      queryParams[key] = value;
+    }


Please note that in real querystring, both key and value are percent-encoded or URL encoded in the URL. For example, the string "5%" will be encoded as "5%25". So to get the actual value of "5%25" (whether it is a key or value in the querystring), you should call a function to decode it.
May I suggest looking up any of these terms, and "How to decode URL encoded string in JS"?

Implemented decoding by calling decodeURIComponent on the string passed

cjyuan · 2025-11-12T00:08:59Z

Sprint-2/implement/tally.js

+    const result = {};
+    for (const item of arr) {
+        result[item] = (result[item] || 0) + 1;
+    }


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.

Implemented Object.create(null) to create an empty object with no inherited properties and test for strings with object methods to confirm there is no unwanted behaviour

cjyuan · 2025-11-12T00:11:22Z

Sprint-2/implement/tally.test.js

+test("should handle array with null and undefined values", () => {
+  expect(tally([null, undefined, null, 'a', undefined])).toEqual({ null: 2, undefined: 2, a: 1 });
+});


What do you expect from the following function call?

tally([null, undefined, null, 'a', undefined, "null", "undefined"]

Implemented test and check for undefined/null vs strings of null/undefined by allocating a separate property for null and undefined values

…ayed on a new line

…urrency; improve readability

…mprove handling of encoded characters in tests

…te keys; update tests for clarity and edge cases

cjyuan · 2025-11-18T22:40:57Z

Sprint-2/implement/lookup.js

+      if (Array.isArray(currency) && typeof country === "string") {
+         result[country]
+           ? (result[country] = [result[country], currency])
+           : (result[country] = currency);
+      }
+
+      if (country.length && currency.length && typeof country === 'string' && typeof currency === 'string') {
+         result[country]
+           ? (result[country] = [result[country], currency])
+           : (result[country] = currency);
+      }


Can you explain why the condition on line 25 needs to check if country.length is non-zero, but the condition on line 19 does not?

The normal use of the ? : operator is, condition ? expr1 : expr2, to conditionally evaluates to one of the expressions.

Lines 20-22 is typically expressed as:

result[country] = result[country] ? [result[country], currency] : currency;

I have combined both of the conditions so that the checks apply to both strings and arrays for currency, and have refactored the ternary as suggested.

Sprint-2/implement/querystring.js

Sprint-2/implement/tally.js

…mprove readability and maintainability

…and key-value pairs; Implement decoding uri component at key value pair level; update tests for consistency with expected string outputs

…into sprint-2

…types; update tests for consistency with new key format

cjyuan

Changes are good.

cjyuan · 2025-11-25T15:02:20Z

Sprint-2/implement/tally.test.js

+test('should return an object', () => {
+    expect(typeof tally([])).toBe('object');
+})
 // Given an empty array
 // When passed to tally
 // Then it should return an empty object
-test.todo("tally on an empty array returns an empty object");
+test("should return an empty object when given an empty array", () => {
+  expect(tally([])).toEqual({});
+});


Is the test on 22-24 necessary?

cjyuan · 2025-11-25T15:29:04Z

Sprint-2/implement/lookup.js

+      if (((Array.isArray(currency) && currency.length)|| (typeof currency === 'string' && currency.length)) 
+        && typeof country === "string" && country.length) {
+        result[country] = result[country] ? [result[country], currency] : currency; 
+      }


Suppose before line 19,

value of country is "A"

value of result[country] is "A1"

value of currency is ["A2", "A3"]

What is your expected value of result[country] after line 22?

i786m and others added 7 commits October 29, 2025 10:53

completed tasks set out in debug folder

d6b4ea3

implement tests and code for contains function

3007268

implement tests and code for lookup

b219369

added querystring tests

5a3a247

implement tally function and corresponding tests for various cases

2556038

refactor parseQueryString to handle multiple equal signs in query par…

da1b806

…ameters

correct invert implementation to swap keys and values; add tests for …

7943541

…various cases

i786m added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 2, 2025

cjyuan reviewed Nov 12, 2025

View reviewed changes

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 Nov 12, 2025

i786m added 4 commits November 17, 2025 16:25

fix logging of ingredients in recipe; ensure each ingredient is displ…

df2c83e

…ayed on a new line

refactor createLookup function to use destructuring for country and c…

1cef373

…urrency; improve readability

refactor parseQueryString to decode query string before processing; i…

34a461a

…mprove handling of encoded characters in tests

refactor tally function to handle null and undefined values as separa…

eabafb7

…te keys; update tests for clarity and edge cases

i786m 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 Nov 18, 2025

i786m requested a review from cjyuan November 18, 2025 20:06

cjyuan reviewed Nov 18, 2025

View reviewed changes

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 Nov 18, 2025

i786m and others added 4 commits November 22, 2025 14:42

refactor createLookup function to simplify currency handling logic; i…

85e40ac

…mprove readability and maintainability

refactor parseQueryString to improve handling of empty query strings …

359063e

…and key-value pairs; Implement decoding uri component at key value pair level; update tests for consistency with expected string outputs

Merge branch 'sprint-2' of https://github.com/i786m/Module-Data-Groups …

f8f1af0

…into sprint-2

refactor tally function to improve key generation for different data …

8d627ff

…types; update tests for consistency with new key format

i786m 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 Nov 25, 2025

i786m requested a review from cjyuan November 25, 2025 13:40

cjyuan reviewed Nov 25, 2025

View reviewed changes

cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 25, 2025

cjyuan added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Nov 25, 2025

i786m mentioned this pull request Nov 26, 2025

ITP-SEP-25| Data groups | Imran Mohamed i786m/My-Coursework-Planner#90

Open

illicitonion closed this Jan 7, 2026

Uh oh!

London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Sprint-2 exercises #809

London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Sprint-2 exercises #809

Uh oh!

Conversation

i786m commented Nov 2, 2025

Learners, PR Template

Changelist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants