-
-
Notifications
You must be signed in to change notification settings - Fork 286
Manchester | 25-ITP-Sep | Mahtem T. Mengstu | Sprint 2 |Sprint-2-courseWork #913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
13acaf4
7f8b224
ba4ef98
6453fe4
cb4642b
9dd1769
8bdd6c5
2c03056
1101ce6
d0665bd
d471bca
e550d17
218caa6
d2985e4
6dff138
0088e75
bd20fa1
3735e91
2f7f0e2
161b871
5cddc23
aab642f
7ef1f9c
ba1bc62
2528d55
8e4807e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,11 @@ | ||
| function contains() {} | ||
| function contains(obj, key) { | ||
| if (typeof obj !== "object" || obj === null || Array.isArray(obj)) { | ||
| return false; | ||
| } | ||
| return key in obj; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider the following two approaches for determining if an object contains a property: Which of these approaches suits your needs better?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thank you, I have come to know that, console.log( propertyName in obj ); inherits prototype properties, |
||
|
|
||
| } | ||
|
|
||
| module.exports = contains; | ||
|
|
||
| // In contains.js function implemented and tested. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,19 +17,46 @@ as the object doesn't contains a key of 'c' | |
| // When passed an object and a property name | ||
| // Then it should return true if the object contains the property, false otherwise | ||
|
|
||
| test("Should return true when key exists in object", () => { | ||
| const obj = { a: 1, b: 2 }; | ||
| expect(contains(obj, "a")).toEqual(true); | ||
| }); | ||
|
|
||
| // Given an empty object | ||
| // When passed to contains | ||
| // Then it should return false | ||
| test.todo("contains on empty object returns false"); | ||
| //test.todo("contains on empty object returns false"); | ||
|
|
||
| test("Should return false when empty object is passed", () => { | ||
| const obj = {}; // empty object | ||
| expect(contains(obj, "a")).toEqual(false); | ||
| }) | ||
|
|
||
| // Given an object with properties | ||
| // When passed to contains with an existing property name | ||
| // Then it should return true | ||
|
|
||
| test("Should return true when object has the property", () => { | ||
| const obj = { a: 5, c: 6, r: 5 }; | ||
| expect(contains(obj, "c")).toEqual(true); | ||
| }); | ||
| // Given an object with properties | ||
| // When passed to contains with a non-existent property name | ||
| // Then it should return false | ||
|
|
||
| test("Should return false when object does not have the property", () => { | ||
| const obj = { a: 5, c: 6, r: 5 }; | ||
| expect(contains(obj, "z")).toEqual(false); // non-existent property name | ||
| }); | ||
| // Given invalid parameters like an array | ||
| // When passed to contains | ||
| // Then it should return false or throw an error | ||
|
|
||
| test("Should return false when invalid parameters are used", () => { | ||
| expect(contains([], "a")).toEqual(false); // array | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arrays are objects in JavaScript, and they do have property names -- just not the same ones as objects. When testing whether the function handles arrays properly, try using a key that an array might
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @cjyuan, I have tried to include test which address that an array might realistically contain. e.g. below |
||
| expect(contains(null, "a")).toEqual(false); // null | ||
| expect(contains(5, "a")).toEqual(false); // number | ||
| expect(contains("hello", "a")).toEqual(false); // string | ||
| }) | ||
|
|
||
| // In contains.test.js tests for contains.test.js passed | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,27 @@ | ||
| function createLookup() { | ||
| // implementation here | ||
| function createLookup(input) { | ||
| if (!Array.isArray(input)) { | ||
| throw new Error("Input must be an array of [country, currency] pairs"); | ||
| } | ||
|
|
||
| const lookup = {}; | ||
|
|
||
| for (const pair of input) { | ||
| if (!Array.isArray(pair) || pair.length !== 2) { | ||
| throw new Error("Each item must be an array of [country, currency]"); | ||
| } | ||
|
|
||
| const [country, currency] = pair; | ||
|
|
||
| if (typeof country !== "string" || typeof currency !== "string") { | ||
| throw new Error("Each item must be an array of [country, currency]"); | ||
| } | ||
|
|
||
| lookup[country] = currency; | ||
| } | ||
|
|
||
| return lookup; | ||
| } | ||
|
|
||
| module.exports = createLookup; | ||
|
|
||
| // In lookup.js function createLookup implemented. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,45 @@ | ||
| function parseQueryString(queryString) { | ||
| const queryParams = {}; | ||
| const parsedParams = {}; // Gets the final key-value pairs | ||
|
|
||
| if (!queryString) return parsedParams; | ||
|
|
||
| // Removes leading '?' if present | ||
|
|
||
| if (queryString.startsWith("?")) { | ||
| queryString = queryString.slice(1); | ||
| } | ||
|
|
||
| if (queryString.length === 0) { | ||
| return queryParams; | ||
| return parsedParams; | ||
| } | ||
| const keyValuePairs = queryString.split("&"); | ||
|
|
||
| for (const pair of keyValuePairs) { | ||
| const [key, value] = pair.split("="); | ||
| queryParams[key] = value; | ||
| // Split the string into individual key-value pairs | ||
| const pairs = queryString.split("&"); | ||
|
|
||
| for (const pair of pairs) { | ||
| if (!pair) continue; // skip empty segments (like from && or trailing &) eg "name=John&&age=30" | ||
|
|
||
| const equalSignIndex = pair.indexOf("="); | ||
|
|
||
| let paramKey, paramValue; | ||
|
|
||
| if (equalSignIndex === -1) { | ||
|
|
||
| // If '=' not found we have a key exists but value is empty | ||
|
|
||
| paramKey = pair; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You missed decode this
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing out that, I have modified it to be with added decode. param key = decodeURIComponent(pair); |
||
| paramValue = ""; | ||
| } else { | ||
| paramKey = decodeURIComponent(pair.slice(0, equalSignIndex)); | ||
| paramValue = decodeURIComponent(pair.slice(equalSignIndex + 1)); | ||
| } | ||
|
|
||
| parsedParams[paramKey] = paramValue; // overwrite previous value if key repeats | ||
| } | ||
|
|
||
| return queryParams; | ||
| return parsedParams; | ||
| } | ||
|
|
||
| module.exports = parseQueryString; | ||
|
|
||
| // In querystring.js function implemented. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,24 @@ | ||
| function tally() {} | ||
| function tally(items) { | ||
| if (!Array.isArray(items)) { | ||
| throw new Error("Input must be an array"); | ||
| } | ||
|
|
||
| if (items.length === 0) { | ||
| return {}; | ||
| } | ||
|
|
||
| const counts = {}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the following function call returns the value you expect? Suggestion: Look up an approach to create an empty object with no inherited properties.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thank you, I tested it and it returns {toString: 'function toString() { [native code] }11'} , as it is getting the inherent property and I have come to know that const counts = Object.create(null); solves the case. |
||
|
|
||
| for (const item of items) { | ||
|
|
||
| // Convert objects and arrays to JSON string | ||
| const key = (typeof item === "object" && item !== null) ? JSON.stringify(item) : item; | ||
| counts[key] = (counts[key] || 0) + 1; | ||
| } | ||
|
|
||
| return counts; | ||
| } | ||
|
|
||
| module.exports = tally; | ||
|
|
||
| // In tally.js function tally() implemented. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make each ingredient to appear on a new line (to meet the specification on line 4)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @cjyuan I have modified "${recipe.ingredients.join("\n")}`) to make ingredients appear on new line.