feat: implement createServer function with case conversion #1005
Conversation
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical functional bug that breaks conversions. In src/createServer.js, the call to convertToCase uses the wrong argument order: on or around the line where you invoke it (look for convertToCase(textToConvert, toCase)), the function should be called as convertToCase(toCase, textToConvert) per the requirement that it accepts (caseName, text). This mismatch prevents the business logic from producing correct results and will cause tests to fail. Please swap the arguments accordingly.
Minor improvements (non-blocking): consider decoding the path text with decodeURIComponent before validation/conversion to handle encoded characters, and ensure your import of the converter is robust if the export shape differs.
You’ve done a solid job on server creation, JSON responses, validation, and error handling—once the call order is fixed, the implementation should meet the requirements. Keep it up!
✨ 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
| } | ||
|
|
||
| // Convert text | ||
| const result = convertToCase(textToConvert, toCase); |
There was a problem hiding this comment.
The call to convertToCase uses the wrong argument order. The business logic function expects (caseName, text) but here you pass (textToConvert, toCase). This contradicts the specification which says: “It accepts two params: case name and text to convert.” Please change this call to convertToCase(toCase, textToConvert) so the converter receives the case first and text second. See the task description for the required signature and expected return shape.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
Hi! Thank you for the review. However, I believe there may be a misunderstanding about the convertToCase function signature. function convertToCase(text, caseName) {
const originalCase = detectCase(text);
const words = toWords(text, originalCase);
const convertedText = wordsToCase(words, caseName);
return { originalCase, convertedText };
}This means the function expects:
This happens because the function receives the text where it expects the case name, causing the wordsToCase function to throw an error. 
|
There was a problem hiding this comment.
Hi! Thank you for the review. However, I believe there may be a misunderstanding about the convertToCase function signature.
Looking at the actual implementation in src/convertToCase/convertToCase.js, the function signature is:
function convertToCase(text, caseName) {
const originalCase = detectCase(text);
const words = toWords(text, originalCase);
const convertedText = wordsToCase(words, caseName);
return { originalCase, convertedText };
}This means the function expects:
First parameter: text (the text to convert)
Second parameter: caseName (the target case name)
The current implementation convertToCase(textToConvert, toCase) is actually correct and matches this signature.
When I tried swapping the arguments as suggested (convertToCase(toCase, textToConvert)), all tests failed with errors like:
Unknown case name: hello_world
Unknown case name: helloWorld
This happens because the function receives the text where it expects the case name, causing the wordsToCase function to throw an error.
I ran npm test with the original order and all tests pass successfully. Could you please verify this? The current implementation appears to be working correctly.
Thank you!
|
@mateacademy-ai-mentor Hi! I've responded to your review comment and kept the original |
vkryvytskyy
requested review from
mateacademy-ai-mentor
and removed request for
mateacademy-ai-mentor
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical blocker in src/createServer.js that breaks the required contract with the business logic. Per the spec, convertToCase must be called with (caseName, text), but the code currently calls convertToCase(textToConvert, toCase) (around the success branch where the conversion happens). This can produce incorrect output or fail tests. Please swap the arguments to convertToCase(toCase, textToConvert) to match the required signature.
Additionally, verify the import shape at the top of the file. If ./convertToCase exports the function directly (module.exports = convertToCase), use const convertToCase = require('./convertToCase'); if it exports an object (module.exports = { convertToCase }), then destructuring is correct. After these fixes, re-run the tests to confirm everything passes.
Everything else in the server looks solid: URL parsing, validations, headers, status codes, and JSON payloads align with the requirements. Keep up the good work—this is very close to completion.
✨ 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
| } | ||
|
|
||
| // Convert text | ||
| const result = convertToCase(textToConvert, toCase); |
There was a problem hiding this comment.
Critical: the converter is invoked with the wrong argument order here. The assignment requires convertToCase(caseName, text). Change this call to pass the case name first: convertToCase(toCase, textToConvert) so the business logic receives (caseName, text).
| 'use strict'; | ||
|
|
||
| const http = require('http'); | ||
| const { convertToCase } = require('./convertToCase'); |
There was a problem hiding this comment.
Verify the import shape here. Ensure require('./convertToCase') returns an object with convertToCase property; if the module exports the function directly (module.exports = convertToCase) you should use const convertToCase = require('./convertToCase');.
|
@mateacademy-ai-mentor review again |
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
vkryvytskyy
requested review from
mateacademy-ai-mentor
and removed request for
mateacademy-ai-mentor
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
vkryvytskyy
requested review from
mateacademy-ai-mentor
and removed request for
mateacademy-ai-mentor
There was a problem hiding this comment.
Review Summary
I am approving your solution. Overall, your server implementation in src/createServer.js aligns well with the requirements: correct URL parsing, validation with aggregated errors, JSON responses with proper headers, and invoking business logic using convertToCase(textToConvert, toCase).
Educational override: One high-severity item to verify so you can learn and progress — please double-check the import/export shape for
./convertToCaseat the top of src/createServer.js. Ifsrc/convertToCase.jsdoesmodule.exports = convertToCase, you should import withconst convertToCase = require('./convertToCase'); if it doesmodule.exports = { convertToCase }, then your current destructuring is correct. A mismatch will makeconvertToCaseundefined at runtime, causing failures when you call it [likely near the line whereconvertToCase(textToConvert, toCase)is invoked].
Minor suggestion: consider guarding against an unexpected req.url being undefined before splitting as a small robustness improvement. Great job meeting the spec on error messages, argument order, and response structure—keep it up!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| 'use strict'; | ||
|
|
||
| const http = require('http'); | ||
| const { convertToCase } = require('./convertToCase'); |
There was a problem hiding this comment.
Check the import shape of ./convertToCase. You currently use destructuring: const { convertToCase } = require('./convertToCase'). If that module does module.exports = convertToCase you must instead use const convertToCase = require('./convertToCase'). A wrong import shape will make convertToCase undefined at runtime (high severity).
| } | ||
|
|
||
| // Convert text | ||
| const result = convertToCase(textToConvert, toCase); |
There was a problem hiding this comment.
The function call convertToCase(textToConvert, toCase) on this line matches the task spec (text first, case second). Keep this argument order, but only after you verify the import shape above so the function is defined at runtime.
3 participants
No description provided.