ITP -Northwest- JAN 2025 | Jan Lo | Module-Data groups | SPRINT 2 | WEEK 7 #506
Conversation
Sprint-2/implement/contains.js
Outdated
| function contains(element, key) { | ||
| const result = element[key]; | ||
|
|
||
| // console.log({ | ||
| // key, | ||
| // typeKey: typeof key, | ||
|
|
||
| // result, | ||
| // typeResult: typeof result | ||
| // }) | ||
|
|
||
| // if undefined | ||
| // return false as doesnt exist | ||
| // else, does exist! | ||
| // return true | ||
|
|
||
| if (result === undefined) return false; | ||
| else return true; | ||
|
|
||
| } |
There was a problem hiding this comment.
This approach is not bulletproof because a property can be assigned an undefined value. For example,
let obj { x: undefined }; // An object with a property named "x", and its value is undefined.
Can you figure out how else to improve the code?
| test("invalid parameters", () => { | ||
| expect(contains(["apple", "orange", "pear"])).toBe (false); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Invalid parameters could also be other types of values. For examples, null, "some string", 123, undefined.
With your current implementation, contains(["apple", "orange", "pear"], "0") and contains("ABC", "0") would return true`.
You would need to add code to your function to check the first parameter is a valid object and is not an array.
Note: array is a kind of objects in JS.
Sprint-2/implement/querystring.js
Outdated
| const value = valueParts.join("="); | ||
| queryParams[key] = value; |
There was a problem hiding this comment.
Your approach works.
Please note that in real querystring, both key and value are percent-encoded or URL encoded in the URL. For example, the string "5%" will be encoded as "5%25". So to get the actual value of "5%25" (whether it is a key or value in the querystring), you should call a function to decode it.
May I suggest looking up any of these terms, and "How to decode URL encoded string in JS"?
cjyuan
added
Reviewed
|
Changes look good. |
|
It is good practice to respond to each reviewer's comment to acknowledge their feedback and to explain any changes made in response.
|
|
It's also a good practice to give a brief description to each PR.
|
cjyuan
added
Complete
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.