Skip to content

solution#429

Open
panasovskyi wants to merge 3 commits intomate-academy:masterfrom
panasovskyi:develop
Open

solution#429
panasovskyi wants to merge 3 commits intomate-academy:masterfrom
panasovskyi:develop

Conversation

@panasovskyi
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 mostly works, but it fails a core requirement for path parsing, which is a blocker.

  • Critical issue (src/createServer.js): When deriving parts, the code uses pathname.slice(1) and then split('/'), which includes empty segments. This violates the requirement that doubled slashes be ignored and that / yields an empty array. For example, / currently produces [''], and //hello//world includes empty strings. Please update the parts computation to filter out empty strings, e.g., const parts = url.pathname.split('/').filter(Boolean); so that /[] and //hello//world['hello','world'].
  • What you did well: You set Content-Type: application/json and used URL + Object.fromEntries(url.searchParams) to produce string query values as required.
  • Minor improvement (optional): Construct a single URL instance and reuse it to avoid double parsing.

Once the empty segment filtering is added, this should meet all requirements. Good work overall—just fix the path parts handling to pass the edge cases.


✨ 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

/* Write your code here */
// Return instance of http.Server class
const server = http.createServer((req, res) => {
const relativePath = new URL(req.url, 'http://localhost').pathname.slice(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.

Using pathname.slice(1) to create relativePath then splitting later can leave empty strings for consecutive slashes or produce [''] for root path. This breaks the requirement that doubled slashes be ignored and that / results in an empty parts array. Consider not relying on slice(1) + raw split without filtering. What should parts be for / and //hello//world? (The description requires [] and ['hello','world'] respectively.)

const relativePath = new URL(req.url, 'http://localhost').pathname.slice(1);
const params = new URL(req.url, 'http://localhost');
const query = Object.fromEntries(params.searchParams);
const parts = relativePath.split('/');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Splitting relativePath with relativePath.split('/') will produce empty segments for consecutive slashes and for an empty string. Update this line to filter out empty strings, for example:

const parts = relativePath.split('/').filter(Boolean);

This ensures / -> [] and //hello//world -> ['hello','world'], matching the task requirements.

const server = http.createServer((req, res) => {
const relativePath = new URL(req.url, 'http://localhost').pathname.slice(1);
const params = new URL(req.url, 'http://localhost');
const query = Object.fromEntries(params.searchParams);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Creating query with Object.fromEntries(params.searchParams) is fine — it yields string values for parameter values (including "" for parameters without a value). One note: if the same query key appears multiple times, Object.fromEntries will keep the last value; if multi-values are important you may need a different approach. Otherwise this satisfies the requirement that query values are strings.

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: APPROVE

Your server meets the requirements: it correctly parses the pathname into parts (ignoring empty segments from doubled slashes), builds a query object with string values, sets Content-Type: application/json, and responds with exactly { parts, query }. It handles root / as an empty parts array and requests without a query as an empty query object.

Minor suggestion (non-blocking): repeated query keys will keep only the last value when using Object.fromEntries(url.searchParams). If you ever need multi-valued parameters, consider aggregating values per key, e.g., mapping url.searchParams into arrays when keys repeat.

Nice work—clean, compliant, and clear implementation.


✨ 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