Skip to content

feat: implement params server#422

Open
mykhailokurochkin wants to merge 2 commits intomate-academy:masterfrom
mykhailokurochkin:develop
Open

feat: implement params server#422
mykhailokurochkin wants to merge 2 commits intomate-academy:masterfrom
mykhailokurochkin:develop

Conversation

@mykhailokurochkin
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Decision: REQUEST_CHANGES

The server is generally well-structured and meets most requirements, but the query parsing has critical issues that can produce incorrect output in common edge cases, which is a blocker for the task’s requirement that the query object be correctly parsed with string values.

Critical issues to fix (src/createServer.js):

  • On empty queries, the current logic sets an invalid entry: at the point where you do params.search.slice(1).split('&') and iterate, an empty search results in [''], leading to query[''] = undefined. This violates the requirement that an empty query should yield {}.
  • URL decoding is not performed. Keys/values like name=John%20Doe are returned encoded, which is incorrect.
  • Parameters without values (e.g., ?flag) result in undefined values instead of empty strings, violating the “all values are strings” requirement.
  • Duplicate keys handling is implicit/accidental. While the last one currently overwrites previous ones, this should be handled intentionally per standard parsing behavior.
  • Not all values are ensured to be strings; some end up as undefined.

How to fix (guidance):

  • Use the WHATWG URL API’s params.searchParams to iterate over parameters. Example approach:
    • Initialize an empty query object.
    • Iterate with for (const [key, value] of params.searchParams); this automatically handles empty queries, decoding, parameters without values (value becomes ""), and duplicate keys (last wins by default if you simply assign).
    • Assign query[key] = String(value) to guarantee string values.

Minor suggestions (non-blocking):

  • Add a brief comment explaining why searchParams is used (edge cases, decoding, duplicate keys).
  • Consider adding simple tests for ?, ?flag, ?a=1&a=2, and encoded values.

You’ve done a solid job on the server structure, pathname parsing, and response formatting. Once the query parsing is updated as above, this will fully meet the requirements. Keep going—you're close!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment on lines +12 to +14
for (const part of params.search.slice(1).split('&')) {
query[part.split('=')[0]] = part.split('=')[1];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query parsing logic here does not handle several required edge cases:

  • If there is no query string, params.search.slice(1) will be an empty string, resulting in [''] after split, so query[''] = undefined will be set.
  • It does not decode URL-encoded keys/values.
  • It does not handle parameters without values (e.g., ?flag).
  • It does not ensure all values are strings.
  • It does not handle duplicate keys according to standard URL parsing behavior.

Consider using params.searchParams (from the WHATWG URL API) to iterate over query parameters. This will handle all these cases correctly and ensure compliance with the requirements.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Great work—your server fully meets the functional and technical requirements.

  • The HTTP server handles all methods and paths correctly and returns a JSON object with exactly parts (array) and query (object).
  • Path parsing removes empty segments from leading/trailing/multiple slashes, and query parsing uses the WHATWG URL API with proper decoding, string values, and last-value-wins behavior for duplicate keys.
  • The entry point starts the server on the correct port and performs a successful self-check via an axios GET.

Minor (non-blocking) suggestion:

  • In src/createServer.js, consider removing the debug log console.log(params.searchParams); (around line 12) for cleaner output in production.

Nice job implementing a clear, robust solution. Keep it up!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants