Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on setting up the server and using URLSearchParams to handle the query parameters!
This submission requires changes because the logic for parsing the URL pathname does not correctly handle double slashes, which is a core requirement. For example, a path like //hello/world is not being parsed into ['hello', 'world'] as expected.
A more robust approach is to always split the pathname by / and then filter out any empty strings from the resulting array. This single strategy will correctly handle all cases, including single slashes, double slashes, and slashes at the beginning or end of the path. After you adjust this logic, the server should meet all the requirements.
✨ 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
src/createServer.js
Outdated
| if (urlArguments[0].includes('//')) { | ||
| parts = urlArguments[0].slice(1).split('//'); | ||
| } else { | ||
| parts = urlArguments[0].slice(1).split('/'); | ||
| } |
There was a problem hiding this comment.
This logic for handling double slashes doesn't cover all cases correctly. For example, a path like //hello/world/123 will be incorrectly parsed into ['', 'hello/world/123'] instead of the required ['hello', 'world', '123'].
A more reliable approach is to split the pathname by / and then filter out any empty string elements from the resulting array. This single strategy will correctly handle single slashes, double slashes, and slashes at the beginning or end of the path.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job implementing the query parameter parsing! That part works perfectly. However, the logic for handling the URL pathname still needs some work as it doesn't handle all the required cases. For example, a path like //hello/world is incorrectly parsed as ['hello/world'] instead of the expected ['hello', 'world']. The current if/else logic is a bit complex and misses this case. A more robust solution, as suggested previously, is to split the pathname by / and then filter out any empty strings from the resulting array. This will correctly handle all variations of slashes (leading, trailing, and double). Keep up the great work, you're almost there!
✨ 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
src/createServer.js
Outdated
| if (urlArguments[0].includes('//')) { | ||
| parts = urlArguments[0].slice(2).split('//'); | ||
| } else { | ||
| parts = urlArguments[0].slice(1).split('/'); | ||
| } |
There was a problem hiding this comment.
This logic for handling pathname parsing is not yet correct. It fails on several cases required by the task, for example, a path like //hello/world will result in ['hello/world'] instead of ['hello', 'world'].
A more robust approach that handles all cases (leading slashes, double slashes, trailing slashes) is to split the path by / and then filter out any resulting empty strings.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this iteration! I'm approving your solution as it now perfectly handles all the requirements. Your updated logic for parsing the URL pathname using split and filter is robust and correctly handles all edge cases, including leading and double slashes. The query parameter parsing is also spot-on. The entire implementation is clean, efficient, and fully meets the task specifications. Well done!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.