West Midlands | ITP-Sept-2025 | Hadi Vahidi | Sprint 2 | Module Data Groups #914
Conversation
…e input is not NaN
|
Your PR's title didn't contain a known region. Please check the expected title format, and make sure your region is in the correct place and spelled correctly. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above. |
HadiVahidi20
changed the title
HadiVahidi20
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Please revert the changes made in the "Sprint-1" folder, as they are not related to Sprint-2 exercise.
There was a problem hiding this comment.
Have you run this script to test your function implementation?
Sprint-2/implement/contains.test.js
Outdated
| // When passed to contains | ||
| // Then it should return false or throw an error | ||
| test("contains returns false for invalid input", () => { | ||
| expect(contains([], "a")).toBe(false); |
There was a problem hiding this comment.
Arrays are objects in JavaScript, and they do have property names -- just not the same ones as objects.
Which keys do arrays have, and how does that affect how reliable your test is?
When testing whether the function handles arrays properly, try using a key that an array might
realistically contain. Otherwise, you might get a passing test even if the function isn't checking for arrays at all.
|
|
||
| const key = pair.substring(0, index); // before the = | ||
| const value = pair.substring(index + 1); // after the = | ||
| queryParams[key] = value; |
There was a problem hiding this comment.
In real query string, both key and value are percent-encoded or URL encoded.
For example,
tags%5B%5D=hello%20world-> key istags[], value ishello world
Can your function handle URL-encoded query string?
Suggestion: Look up "How to decode a URL-encoded string in JavaScript".
Sprint-2/implement/tally.js
Outdated
| throw new Error("Input must be an array"); | ||
| } | ||
|
|
||
| const counts = {}; |
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
HadiVahidi20
added
Needs Review
illicitonion
added
Complete
|
Well done! |
3 participants
Learners, PR Template
Self checklist
Changelist
I have done every thing in sprint 2
Questions
no question thank you so much