-
-
Notifications
You must be signed in to change notification settings - Fork 221
Birmingham | ITP-Sep-25 | Ahmad Ehsas | sprint 2 | Data groups #805
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 4 commits
96e54cd
d577680
3ecbe9f
0467d03
a9a5914
d5ca4ab
ce7733e
73e8544
f512318
22c4094
1231d9c
aa42568
ef820ad
898abfc
2bc209b
d0788b8
9940e55
ae98aa1
268774e
9c2f2f8
1f7a23d
fa7c649
e165bfe
28a4d76
da2a8c5
1cc69af
954d882
99d8f1f
f0cfdc0
ee378d5
f1c47e4
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 |
|---|---|---|
|
|
@@ -46,9 +46,11 @@ test("contains on an array returns false", () => { | |
| }); | ||
|
|
||
| test("contains on an array of length returns true", () => { | ||
| expect(contains([1, 2, 3, 4], "length")).toBe(true); | ||
| // expect(contains([1, 2, 3, 4], "length")).toBe(true); | ||
| expect(contains(null, "a", "length")).toBe(true); | ||
| }); // the test returns true because 'length' is a property of the array object | ||
|
|
||
| test("contains given on array as input return false", () => { | ||
| expect(contains([1, 2, 3], "a")).toBe(false); | ||
| // expect(contains([1, 2, 3], "a")).toBe(false); | ||
| expect(contains(1234, "a")).toBe(false); | ||
| }); | ||
|
Comment on lines
44
to
60
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. Do these function calls return what you expect?
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. My function checks for its own property on an object. For the first input `contains([1, 2, 3], "0"); return true because arrays are objects and "0" is a property key.
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. In that case you may want to update the test descriptions on lines 44 and 52.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,18 @@ function parseQueryString(queryString) { | |
| if (queryString.length === 0) { | ||
| return queryParams; | ||
| } | ||
|
|
||
| const keyValuePairs = queryString.split("&"); | ||
|
|
||
| for (const pair of keyValuePairs) { | ||
| const [key, value] = pair.split(/=(.+)/); // We use regex to split only at first. | ||
| const [key, value] = pair.split(/=(.*)/); // We use regex to split only at first. | ||
| queryParams[key] = value; | ||
|
|
||
| } | ||
|
Comment on lines
7
to
13
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 this function call return what you expect? In real query string, both
Can your function handle URL-encoded query string? Suggestion: Look up "How to decode a URL-encoded string in JavaScript". |
||
|
|
||
| return queryParams; | ||
| } | ||
| console.log(parseQueryString("A=")); | ||
| console.log(decodeURIComponent("id%3D5")); | ||
|
|
||
| module.exports = parseQueryString; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,10 @@ console.log(invert({ a: 1, b: 2 })); // {1: "a", 2: "b"} | |
| module.exports = invert; | ||
|
|
||
| // a) What is the current return value when invert is called with { a : 1 } | ||
| // The current return value is {1: "a"} | ||
| // The current return value is {'1': 'a'} | ||
|
|
||
| // b) What is the current return value when invert is called with { a: 1, b: 2 } | ||
| // The current return value is {1: "a", 2: "b"} | ||
| // The current return value is {'1': 'a', '2': 'b'} | ||
|
||
|
|
||
| // c) What is the target return value when invert is called with {a : 1, b: 2} | ||
| // The target return value is {1: "a", 2: "b"} | ||
|
|
||
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.
Leaving commented out unused code can make code review difficult. Can you keep the the code clean by removing all unnecessary code?

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.
I removed the unnecessary code.