-
-
Couldn't load subscription status.
- Fork 164
Glasgow | 25-ITP-SEP | Shreef Ibrahim | Sprint 2| Coursework #793
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
base: main
Are you sure you want to change the base?
Changes from all commits
ceb17df
4d1d8ac
04fa106
cd4cb2b
629cd62
36e36ef
8bd2a03
62191e1
f324295
8fe46af
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 |
|---|---|---|
|
|
@@ -11,6 +11,12 @@ const author = { | |
| alive: true, | ||
| }; | ||
|
|
||
| for (const value of author) { | ||
| // for (const value of author) { | ||
| // console.log(value); | ||
| // } | ||
| // It will log syntax Error because Objects themselves aren’t directly iterable Unlike arrays we need to converted | ||
| // and then we can loop | ||
|
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. Very good explanation — you clearly identified why the original loop didn't work and how to resolve it. @shreefAhmedM |
||
|
|
||
| for (const value of Object.values(author)) { | ||
| console.log(value); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,19 @@ | |
| // This program should log out the title, how many it serves and the ingredients. | ||
| // Each ingredient should be logged on a new line | ||
| // How can you fix it? | ||
|
|
||
| const recipe = { | ||
| title: "bruschetta", | ||
| serves: 2, | ||
| ingredients: ["olive oil", "tomatoes", "salt", "pepper"], | ||
| }; | ||
|
|
||
| console.log(`${recipe.title} serves ${recipe.serves} | ||
| ingredients: | ||
| ${recipe}`); | ||
| // console.log(`${recipe.title} serves ${recipe.serves} | ||
| // ingredients: | ||
| // ${recipe}`); | ||
| // When you using ${recipe} in a template literal javaScript tries to convert the entire object to a string | ||
| // it will log [object Object]. | ||
| console.log(`${recipe.title} serves ${recipe.serves}`); | ||
| console.log("ingredients"); | ||
| recipe.ingredients.forEach((ingredient) => { | ||
|
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. Using forEach is a clean and readable way to loop through the ingredients. Good choice. |
||
| console.log(ingredient); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| function contains() {} | ||
|
|
||
| function contains(obj, prop) { | ||
|
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. @shreefAhmedM we can make the argument prop more descriptive |
||
| return obj && typeof obj === "object" && prop in obj; | ||
|
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. I like that you safeguard against non-object inputs by checking typeof obj === "object". It makes the function more robust and prevents errors when unexpected values are passed in. |
||
| } | ||
| // console.log(contains(({ a: 1, b: 2 }, "c"))); | ||
| module.exports = contains; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,20 +16,31 @@ as the object doesn't contains a key of 'c' | |
| // Given a contains function | ||
| // When passed an object and a property name | ||
| // Then it should return true if the object contains the property, false otherwise | ||
| test("contains returns true for existing property", () => { | ||
| expect(contains({ a: 1, b: 2 }, "a")).toBe(true); | ||
| }); | ||
|
|
||
| // Given an empty object | ||
| // When passed to contains | ||
| // Then it should return false | ||
| test.todo("contains on empty object returns false"); | ||
|
|
||
| test("contains on empty object returns false", () => { | ||
| expect(contains({}, "a")).toBe(false); | ||
| }); | ||
| // Given an object with properties | ||
| // When passed to contains with an existing property name | ||
| // Then it should return true | ||
|
|
||
| test("Given an object with an existing property should return true", () => { | ||
| expect(contains({ a: 1, k: 2 }, "k")).toBe(true); | ||
| }); | ||
| // Given an object with properties | ||
| // When passed to contains with a non-existent property name | ||
| // Then it should return false | ||
|
|
||
| test("Given an object with none-existing property should return false", () => { | ||
|
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. Very Good! |
||
| expect(contains({ a: 1, b: 2 }, "c")).toBe(false); | ||
| }); | ||
| // Given invalid parameters like an array | ||
| // When passed to contains | ||
| // Then it should return false or throw an error | ||
| test("Given an object with an existing property should return true", () => { | ||
|
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. @shreefAhmedM Good test — small mismatch: the acceptance criteria describe invalid parameters (like an array), but the test title says it's testing an object with an existing property. |
||
| expect(contains(["k", "a"], "k")).toBe(false); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,15 @@ | ||
| function createLookup() { | ||
| // implementation here | ||
| function createLookup(nestedArr) { | ||
|
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. Good job — this works and produces the correct result! 👍 |
||
| const noneNesArr = nestedArr.flat(Infinity); | ||
| const result = {}; | ||
| for (let i = 0; i < noneNesArr.length; i += 2) { | ||
| const country = noneNesArr[i]; | ||
| const currency = noneNesArr[i + 1]; | ||
| result[country] = currency; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| createLookup([ | ||
|
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. We don’t need to call |
||
| ["US", "USD"], | ||
| ["CA", "CAD"], | ||
| ]); | ||
| module.exports = createLookup; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,23 @@ | ||
| const createLookup = require("./lookup.js"); | ||
|
|
||
| test.todo("creates a country currency code lookup for multiple codes"); | ||
|
|
||
| /* | ||
| Create a lookup object of key value pairs from an array of code pairs | ||
| Acceptance Criteria: | ||
| Given | ||
| - An array of arrays representing country code and currency code pairs | ||
| e.g. [['US', 'USD'], ['CA', 'CAD']] | ||
| When | ||
| - createLookup function is called with the country-currency array as an argument | ||
| Then | ||
| - It should return an object where: | ||
| - The keys are the country codes | ||
| - The values are the corresponding currency codes | ||
| Example | ||
| Given: [['US', 'USD'], ['CA', 'CAD']] | ||
| When | ||
| createLookup(countryCurrencyPairs) is called | ||
| Then | ||
| It should return: | ||
| { | ||
| 'US': 'USD', | ||
| 'CA': 'CAD' | ||
| } | ||
| */ | ||
| // Create a lookup object of key value pairs from an array of code pairs | ||
|
|
||
| // Acceptance Criteria: | ||
|
|
||
| // Given | ||
| // - An array of arrays representing country code and currency code pairs | ||
| // e.g. [['US', 'USD'], ['CA', 'CAD']] | ||
| test("creates a country currency code lookup for multiple codes", () => { | ||
| expect( | ||
| createLookup([ | ||
| ["US", "USD"], | ||
| ["CA", "CAD"], | ||
| ]) | ||
| ).toEqual({ US: "USD", CA: "CAD" }); | ||
| }); | ||
|
|
||
| test("creates a country currency code with mixed nesting and extra layers lookup for multiple codes", () => { | ||
|
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. Good thinking outside the box! 👏 But this test with extra nested arrays isn’t part of the requirement — the function only needs to handle a simple array of [country, currency] pairs. |
||
| expect( | ||
| createLookup([["US", "USD"], [["CA", "CAD"]], [[["JP", "JPY"]]]]) | ||
| ).toEqual({ US: "USD", CA: "CAD", JP: "JPY" }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,22 @@ function parseQueryString(queryString) { | |
| if (queryString.length === 0) { | ||
| return queryParams; | ||
| } | ||
|
|
||
| const keyValuePairs = queryString.split("&"); | ||
|
|
||
| for (const pair of keyValuePairs) { | ||
| const [key, value] = pair.split("="); | ||
| queryParams[key] = value; | ||
| const indexOfEquals = pair.indexOf("="); | ||
|
|
||
| if (indexOfEquals === -1) { | ||
| queryParams[pair] = ""; | ||
| } else { | ||
| const key = pair.slice(0, indexOfEquals); | ||
| const value = pair.slice(indexOfEquals + 1); | ||
| queryParams[key] = value; | ||
| } | ||
| } | ||
|
|
||
|
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. Nice work! ✅ This handles values containing = correctly and covers keys with no value. |
||
| return queryParams; | ||
| } | ||
|
|
||
| console.log(parseQueryString("equation=x=y+1")); | ||
|
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. Do we need to call |
||
| module.exports = parseQueryString; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,40 @@ | |
| // Below is one test case for an edge case the implementation doesn't handle well. | ||
| // Fix the implementation for this test, and try to think of as many other edge cases as possible - write tests and fix those too. | ||
|
|
||
| const parseQueryString = require("./querystring.js") | ||
|
|
||
| const parseQueryString = require("./querystring.js"); | ||
| test("parses querystring values containing =", () => { | ||
| expect(parseQueryString("equation=x=y+1")).toEqual({ | ||
| "equation": "x=y+1", | ||
| equation: "x=y+1", | ||
| }); | ||
| }); | ||
| test("parses simple key=value pairs", () => { | ||
| expect(parseQueryString("name=John&age=30")).toEqual({ | ||
| name: "John", | ||
| age: "30", | ||
| }); | ||
| }); | ||
|
|
||
| test("parses value containing '='", () => { | ||
|
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. The tests "parses value containing '='” and "parses querystring values containing =" are very similar. |
||
| expect(parseQueryString("equation=x=y+1")).toEqual({ | ||
| equation: "x=y+1", | ||
| }); | ||
| }); | ||
|
|
||
| test("handles keys with no value", () => { | ||
| expect(parseQueryString("flag&key=value")).toEqual({ | ||
| flag: "", | ||
| key: "value", | ||
| }); | ||
| }); | ||
|
|
||
| test("handles multiple '=' in value", () => { | ||
| expect(parseQueryString("data=key=value=extra")).toEqual({ | ||
| data: "key=value=extra", | ||
| }); | ||
| }); | ||
|
|
||
| test("handles keys with empty values", () => { | ||
| expect(parseQueryString("name=")).toEqual({ | ||
| name: "", | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,11 @@ | ||
| function tally() {} | ||
|
|
||
| function tally(arr) { | ||
| const output = {}; | ||
| if (!Array.isArray(arr)) { | ||
| throw new Error("Input must be an array"); | ||
| } | ||
| arr.forEach((element) => { | ||
| output[element] = (output[element] || 0) + 1; | ||
| }); | ||
| return output; | ||
| } | ||
| module.exports = tally; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,16 +19,26 @@ const tally = require("./tally.js"); | |
| // Given a function called tally | ||
| // When passed an array of items | ||
| // Then it should return an object containing the count for each unique item | ||
|
|
||
| test("Given a function tally(['a']), target output should be { a: 1 }", () => { | ||
|
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. The test title and implementation don’t align. |
||
| expect(tally(["a", "a", "a"])).toEqual({ a: 3 }); | ||
| }); | ||
| // 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"); | ||
|
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. The test.todo is only a placeholder. Once you implement the real test for an empty array, you can remove this line. |
||
|
|
||
| test("tally on an empty array returns an empty object, target output should be {}", () => { | ||
| expect(tally([])).toEqual({}); | ||
| }); | ||
| // Given an array with duplicate items | ||
| // When passed to tally | ||
| // Then it should return counts for each unique item | ||
| test("Given a function tally(['a']), target output should be { a: 1 }", () => { | ||
|
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. We can make the test name align with the test requirement |
||
| expect(tally(["a", "a", "b", "c"])).toEqual({ a: 2, b: 1, c: 1 }); | ||
| }); | ||
|
|
||
| // Given an invalid input like a string | ||
| // When passed to tally | ||
| // Then it should throw an error | ||
| test("Given an invalid input like a string,Then it should throw an error", () => { | ||
| expect(() => tally("string")).toThrow("Input must be an array"); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,3 +26,17 @@ | |
| 3. Order the results to find out which word is the most common in the input | ||
| */ | ||
| function countWords(str) { | ||
| const obj = {}; | ||
| let tidyStr = str.replace(/[!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~]/g, ""); | ||
| let arr = tidyStr.toLowerCase().split(" "); | ||
|
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. Just thinking — splitting by " " only handles regular spaces. It won’t handle tabs (\t) or multiple consecutive spaces correctly. Can we think of a way to handle this? |
||
| arr.forEach((element) => { | ||
| if (obj[element]) { | ||
| obj[element] += 1; | ||
| } else { | ||
| obj[element] = 1; | ||
| } | ||
| }); | ||
| return obj; | ||
| } | ||
| // countWords("you and mE anD you me! me"); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,8 +19,11 @@ function calculateMode(list) { | |
|
|
||
| freqs.set(num, (freqs.get(num) || 0) + 1); | ||
| } | ||
| return highestFreg(freqs); | ||
| } | ||
|
|
||
| // Find the value with the highest frequency | ||
| // Find the value with the highest frequency | ||
| function highestFreg(freqs) { | ||
|
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. @shreefAhmedM Consider renaming |
||
| let maxFreq = 0; | ||
| let mode; | ||
| for (let [num, freq] of freqs) { | ||
|
|
@@ -32,5 +35,4 @@ function calculateMode(list) { | |
|
|
||
| return maxFreq === 0 ? NaN : mode; | ||
| } | ||
|
|
||
| module.exports = calculateMode; | ||
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.
Nice work @shreefAhmedM!
I think we can simplify the comment a bit.