LONDON | 25-ITP-MAY | TEWODROS BEKERE | M-D-G | SPRINT-2 #674
Conversation
…2/debug/address.js
…print-2/author.js
…le on it for sprint 2
…nal to fix that i updated the package.jason
…ccuracy for M-D-G/Sprint-2/implement/lookup.js
… pairs and update tests for accuracy for M-D-G/Sprint-2/implement/querystring.js
…ing tests for M-D-G/Sprint-2/implement/tally.js
… and add test cases for M-D-G/Sprint-2/interpret/invert.js
iteddy16
changed the title
iteddy16
added
📅 Sprint 2
iteddy16
changed the title
iteddy16
changed the title
a-robson
added
Review in progress
| const keyValuePairs = queryString.split("&"); | ||
|
|
||
| for (const pair of keyValuePairs) { |
There was a problem hiding this comment.
How will code cope with invalid input like &name=Zadie& or city=London&&year=2025?
| key = decodeURIComponent(pair.substring(0, index)); | ||
| value = decodeURIComponent(pair.substring(index + 1)); | ||
| } else { | ||
| key = decodeURIComponent(pair); |
There was a problem hiding this comment.
What will happen if string passed to decodeURIComponent is not a well-formed URI?
| if (obj.hasOwnProperty(key)) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Do we need an if/else here? How could we simplify this?
| function tally(items) { | ||
| if (!Array.isArray(items)) { | ||
| throw new Error("Input must be an array"); | ||
| } | ||
|
|
||
| const counts = {}; | ||
|
|
||
| for (let i = 0; i < items.length; i++) { | ||
| const item = items[i]; | ||
| if (counts[item]) { | ||
| counts[item]++; | ||
| } else { | ||
| counts[item] = 1; | ||
| } | ||
| } | ||
|
|
||
| return counts; | ||
| } |
There was a problem hiding this comment.
Your current solution works well! For future reference, this is a common use case for the reduce() method, which is designed specifically for accumulating values from arrays. If you're interested in exploring an alternative approach, reduce() could make this more concise.
No changes needed—just something to keep in mind for similar problems.
| const { test } = require("picomatch"); | ||
|
|
There was a problem hiding this comment.
Solution is good and explanations are clear. However I'm not sure why you are importing picomatch. Please put your tests in a separate file - invert.test.js
a-robson
added
Reviewed
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.