first update#4975
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still explicit checklist and requirement violations, even though the core parsing logic and output are correct. Your convertToObject function correctly turns the style string into an object and skips empty declarations, so the main behavior is solid.
Critical issues that must be fixed:
- You are using a
forloop over the split declarations (insrc/convertToObject.js), which violates the requirement to avoid loops and use iteration methods instead ([CHECKLIST ITEM #5]). Refactor that loop to use something likeforEachorreduceon the array of declarations while preserving the same logic. - The variable names
rawobjandarraynewviolate the naming rule to avoid genericobj/array-style names ([CHECKLIST ITEM #2]). Rename them to something descriptive of their purpose, such asstylesObjectfor the accumulator object anddeclarationPartsfor thekey:valuesplit pieces.
Once you replace the loop with an iteration method and update those variable names, your solution should fully meet both the functional requirements and the checklist. You’re very close—your main logic is good, it just needs these structural and naming adjustments to align with the task rules.
✨ 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
| const rawobj = {}; | ||
| const splittedobj = sourceString.split(';'); | ||
|
|
||
| for (let i = 0; i < splittedobj.length; i++) { |
There was a problem hiding this comment.
This for loop violates checklist item #5: Avoid using loops in this task, use iteration methods instead. Refactor this logic using array iteration methods such as forEach or reduce.
| */ | ||
| function convertToObject(sourceString) { | ||
| // write your code here | ||
| const rawobj = {}; |
There was a problem hiding this comment.
The variable name rawobj violates checklist item #2: Avoid variable namings like array,object arr, obj, resultbecause it effectively reads asraw object. Use a more descriptive name that reflects its purpose, like stylesObject`.
| continue; | ||
| } | ||
|
|
||
| const arraynew = newItem.split(':'); |
There was a problem hiding this comment.
The variable name arraynew violates checklist item #2: Avoid variable namings like array,object arr, obj, result. Choose a name that describes the content, such as declarationParts`.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is still a direct checklist violation in your code, even though the core functionality is correct and complete. Specifically, in src/convertToObject.js, the reduce accumulator is named rawobj, which conflicts with the naming rule that forbids obj-style names and requires descriptive variable names [CHECKLIST ITEM #2]. This is treated as a critical requirement for this task, not just a cosmetic preference.
To fix this, rename rawobj in your reducer to something descriptive of its purpose, such as stylesObject or stylesMap, and update all its usages inside the callback accordingly (for example, acc.stylesObject[key] = value style usage, depending on your structure). Once you use a clear, intention-revealing name for the accumulator, your solution will align with both the functional requirements and the checklist. Functionality-wise, you did well: the parsing behaviour, handling of whitespace, skipping invalid declarations, and use of reduce instead of loops all look solid. After this small but required naming adjustment, your implementation should be fully acceptable.
✨ 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
| .split(';') | ||
| .map((item) => item.trim()) | ||
| .filter((item) => item.includes(':')) | ||
| .reduce((rawobj, currentItem) => { |
There was a problem hiding this comment.
This violates checklist item #2: "[CODE STYLE] - Avoid variable namings like array , object arr, obj, result. It will decrease code readability". The accumulator name rawobj is an obj-style name; rename it to something descriptive of its purpose (e.g., stylesObject or stylesMap) and update its uses inside the reducer.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because convertToObject now fully matches the task requirements and handles all the tricky cases correctly. Your function cleanly parses the style string into an object, correctly keeps multi-word values (like 1px solid #e8e8e8), and properly supports vendor-prefixed properties such as -webkit- and -moz-. You also did well using array methods (split, filter, reduce) instead of explicit loops, and your accumulator name stylesObject is clear and avoids the discouraged generic names. The handling of irregular whitespace and skipping of empty or semicolon-only lines is robust and avoids creating invalid keys. Overall, there are no remaining checklist or style issues, and the code is both readable and aligned with the project guidelines. Nicely done—this is a solid, production-quality implementation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
logic is easy, but comparative conditionals are so hard idk this callbacks will kill me one day.