Manchester | 26-ITP-Jan | Abdu Hassen | Sprint 2| Data-Grouping#1121
Manchester | 26-ITP-Jan | Abdu Hassen | Sprint 2| Data-Grouping#1121Abduhasen wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
Abduhasen
added
📅 Sprint 2
Sprint-2/debug/author.js
Outdated
|
|
||
| for (const value of author) { | ||
| console.log(value); | ||
| for (const property in author) { |
There was a problem hiding this comment.
Can also name the variable key -- shorter name.
Sprint-2/debug/recipe.js
Outdated
| for (const element of recipe.ingredients) { | ||
| console.log(element); | ||
| } |
There was a problem hiding this comment.
Your code works.
Here is an alternative worth exploring:
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.
| if ( | ||
| typeof targetObject !== "object" || | ||
| Object.keys(targetObject).length === 0 || | ||
| Array.isArray(targetObject) | ||
| ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
What would happen when targetObject is null?
The check on line 4 is optional. If searchTerm is not a key of the object, the remaining code would also return false.
Sprint-2/implement/contains.js
Outdated
| for (const property in targetObject) { | ||
| if (targetObject[property] === searchTerm || property === searchTerm) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
-
The function is supposed to check only if it contains
searchTermas its key. There is no need to compare its value. -
Can also explore
Object.hasOwn().
Sprint-2/implement/contains.js
Outdated
| } | ||
| return false; | ||
| } | ||
| console.log(contains(["a", "b", "c"], "d")); |
There was a problem hiding this comment.
Should delete all unnecessary code to keep the code clean.
Sprint-2/implement/contains.test.js
Outdated
| test("the function should return false when invalid parameter like an array is passed", () => { | ||
| expect(contains([10, 23, 34, 45], 45)).toBe(false); | ||
| expect(contains("hello", "hi")).toBe(false); | ||
| expect(contains(["green", "yellow", "rad"], "green")).toBe(false); |
There was a problem hiding this comment.
This test does not yet confirm that the function correctly returns false when the first argument is an array.
This is because contains([10, 23, 34, 45], 45) could also return false simply because 45 is not a key of the array.
Arrays are objects, with their indices acting as keys. A proper test should use a valid key to ensure the function returns false specifically because the input is an array, not because the key is missing.
| for (const pair of keyValuePairs) { | ||
| const [key, value] = pair.split("="); | ||
| const equalityPosition = pair.indexOf("="); | ||
| if (equalityPosition == -1) { | ||
| continue; | ||
| } | ||
| const key = pair.slice(0, equalityPosition); | ||
| const value = pair.slice(equalityPosition + 1); | ||
| queryParams[key] = value; | ||
| } | ||
|
|
||
| return queryParams; | ||
| } |
There was a problem hiding this comment.
These arguments are also valid query strings:
parseQueryString("key1=value1&=&key2=value2")
parseQueryString("key=")
parseQueryString("key1=value1&key2")
parseQueryString("=value")
parseQueryString("key1=value1&&key2=value2")
Sprint-2/implement/tally.js
Outdated
| if (myArray.length === 0) { | ||
| return {}; | ||
| } else if (typeof myArray === "string") { | ||
| throw Error; | ||
| } |
There was a problem hiding this comment.
-
Why only throw an error when
myArrayis a string? Why not do the same for other types of data? -
Why not throw an error when
myArrayis an empty string?
Sprint-2/implement/tally.js
Outdated
| } else if (typeof myArray === "string") { | ||
| throw Error; | ||
| } | ||
| let tallyObject = {}; |
There was a problem hiding this comment.
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.
cjyuan
added
Reviewed
Abduhasen
added
Needs Review
2 participants
Learners, PR Template
Self checklist
Changelist
-debugging of codes
-implementing of codes
-interpreting of codes
Questions
N/A