London|25-ITP-September|Alexandru Pocovnicu|Sprint 2 by alexandru-pocovnicu · Pull Request #814 · CodeYourFuture/Module-Data-Groups

alexandru-pocovnicu · 2025-11-03T14:16:22Z

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

implemented functions using TDD

…n a single line

… properties and invalid parameters

…d consistency

…ability

… code mapping

…lid inputs

… with input validation

…rs and key-value parsing

github-actions · 2025-11-03T14:16:37Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-11-03T14:16:42Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-11-03T14:16:48Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-11-03T14:16:48Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-11-03T14:16:49Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

github-actions · 2025-11-03T14:16:50Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

takanocap

@alexandru-pocovnicu Good effort on Sprint 2. However, there a few comments for you to address.

Sprint-2/debug/author.js

takanocap · 2025-11-14T12:42:55Z

Sprint-2/debug/recipe.js

+console.log(`${recipe.title}, serves ${recipe.serves}, ingredients:`);
+
+for (const ingredient of recipe.ingredients) {
+  console.log(ingredient);


can this be done with an array method which may be simpler/apt in this scenario?

takanocap · 2025-11-14T13:14:08Z

Sprint-2/implement/contains.js

+  if (typeof obj !== "object" || obj === null || Array.isArray(obj)) {
+    throw new Error("Parameter is not an object literal");
+  }
+  if (Object.keys(obj).length === 0) {


is there a way to make fewer checks so we you don't repeat the same check logically?

takanocap · 2025-11-14T13:17:32Z

Sprint-2/implement/contains.js

+    return true;
+  }
+
+  return false;


is there a way we can reduce repetition? what is the DRY principle?

takanocap · 2025-11-14T13:37:43Z

Sprint-2/implement/lookup.js

-  // implementation here
+function createLookup(countryCurrency) {
+  let countryCurrencyPairs = {};
+  for (const pair of countryCurrency) {


what happens if you test this solution with an argument which is not an object?

takanocap · 2025-11-14T13:45:50Z

Sprint-2/implement/querystring.js

@@ -3,14 +3,30 @@ function parseQueryString(queryString) {
  if (queryString.length === 0) {


is there a better way to check this and exit or handle the error if its not a valid querystring?

takanocap · 2025-11-14T14:03:02Z

Sprint-2/implement/querystring.js

  for (const pair of keyValuePairs) {
-    const [key, value] = pair.split("=");
-    queryParams[key] = value;
+    if(pair===""){


can this be shorter or tested in a different way ? investigate how to test if string has a empty or falsy value

so should i change my tests also? if i use array destructuring none of the test are passing anymore

yes please because thats what will typically happen in a work environment

if i use array destructuring it doesn't pass the tests as some of the values have "=" in them, or at least i don't know how to do it. some pointers would be much appreciated , thank you

…stency

…dling key-value pairs

illicitonion · 2025-12-19T14:37:15Z

Sprint-2/implement/contains.test.js

+test("if the object contains the property, return 'true'", () => {
+  expect(contains({ a: 1, "a,s,3": "op" }, ["a", "s", 3])).toEqual(true);
+});


I feel like this is slightly surprising behaviour - the object doesn't really contain that array, it contains a stringification of that array. But this works, so I guess it is ok to test for...

I would probably use a different name for the test, though, to make it clear how it's different from the test on line 19.

illicitonion · 2025-12-19T14:39:24Z

Sprint-2/implement/lookup.test.js

+test("throw error if the argument is not an object", () => {
+  expect(() => createLookup("notAnObject")).toThrow(
+    "Argument must be an array"


It's a bit odd that the test name here talks about objects, the string says notAnObject, but the error message is about being an array?

illicitonion · 2025-12-19T14:40:42Z

Sprint-2/interpret/invert.js

+//[[ "a", 1], ["b", 2]]

 // c) What is the target return value when invert is called with {a : 1, b: 2}
+//{"1":a,"2":b}


What is the type of a and b here? Are they strings/numbers/booleans/variables/something else?

github-actions · 2025-12-19T14:41:18Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

github-actions · 2025-12-19T14:41:24Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

alexandru-pocovnicu added 21 commits October 24, 2025 10:12

Add initial test file for mean calculations

71fa218

Add mean.js file for mean calculations

927769b

Remove mean.js and mean.test.js files

a393af9

Fix address object access to correctly log house number

256a0bb

Fix loop iteration to correctly log author object properties

93dca5e

Fix recipe logging to display ingredients on separate lines

538d536

Fix recipe logging format to display title, serves, and ingredients i…

608a2f2

…n a single line

Add test case for contains function with empty object

3a82eed

Implement contains function to check for property existence in an object

dbd5551

Add additional test cases for contains function to cover non-existent…

1769faf

… properties and invalid parameters

Add error handling for non-object parameters in contains function

8fd279f

Refactor test cases for contains function for improved readability an…

1ff152e

…d consistency

Fix formatting inconsistencies in contains function for improved read…

2982bf2

…ability

Implement test for createLookup function to validate country-currency…

cb3b23c

… code mapping

Implement createLookup function to map country to currency

e56a2bf

Add tests for tally function to validate behavior with empty and inva…

3f4a115

…lid inputs

Implement tally function to count occurrences of elements in an array…

493e3f1

… with input validation

Refactor query string tests for improved clarity and structure

577ae89

Refactor parseQueryString function for improved handling of empty pai…

99caad7

…rs and key-value parsing

Fix invert function to correctly swap keys and values in the object

f998538

Add test for invert function to verify key-value swapping behavior

e098bba

alexandru-pocovnicu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 3, 2025

takanocap reviewed Nov 14, 2025

View reviewed changes

changed loop to log just the values

b1aa75d

alexandru-pocovnicu added 8 commits November 24, 2025 08:43

changed the function to use an array method

55571b5

removed redundant code

b8bdddf

deleted repeating code

a7d4b52

Add test for error handling when argument is not an array

4464835

Add input validation to createLookup function

a236a49

added check to see if the querryString is a string or falsy

532bb55

Refactor parseQueryString function for improved readability and consi…

af31865

…stency

Refactor parseQueryString function for clarity and consistency in han…

7a17d26

…dling key-value pairs

alexandru-pocovnicu mentioned this pull request Dec 4, 2025

25-ITP-September| Data groups | Alexandru Pocovnicu alexandru-pocovnicu/My-Coursework-Planner#71

Open

illicitonion reviewed Dec 19, 2025

View reviewed changes

illicitonion added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Dec 19, 2025

alexandru-pocovnicu closed this by deleting the head repository Jan 1, 2026

		@@ -3,14 +3,30 @@ function parseQueryString(queryString) {
		if (queryString.length === 0) {

Uh oh!

Conversation

alexandru-pocovnicu commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

takanocap left a comment

Choose a reason for hiding this comment

Uh oh!

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!

github-actions bot commented Dec 19, 2025

Uh oh!

github-actions bot commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexandru-pocovnicu commented Nov 3, 2025 •

edited

Loading