task resolved#4977
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical blockers that prevent your solution from fully meeting the task requirements. The main functional issue is that your parsed values still include the trailing semicolon (e.g. '#fff;'), so the resulting object will not strictly equal the expected reference object; you need to trim and then remove a final ; from each value before assigning it. Additionally, the for...of loop over the formatted lines violates the requirement to use iteration methods instead [CHECKLIST ITEM #5]; refactor that part to use something like reduce or forEach on the lines array. Your current approach also mutates an external accumulator object while using iteration logic, which conflicts with the guideline against mutating existing objects within non-forEach iteration methods [CHECKLIST ITEM #1]; prefer building and returning a new object inside a reduce callback rather than modifying one defined outside. Finally, the accumulator is named result, which goes against the naming checklist [CHECKLIST ITEM #2]; choose a more descriptive name such as stylesObject or stylesMap to improve readability. Once you fix the semicolon handling, switch the loop to an appropriate iteration method, avoid external mutation, and improve the accumulator naming, your solution should satisfy both the functional and checklist constraints—your overall parsing approach is good, so you’re close to a complete solution.
✨ 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
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical issues that break required behavior and violate explicit checklist rules. Functionally, your value parsing does not reliably remove trailing semicolons from all declarations, which means convertToObject(complexStylesString) may not strictly equal the reference object as required; you should trim the value and explicitly strip a final ; before assigning. The current approach of splitting the whole string by ';' and then by ':' can also mis-handle the given multiline complexStylesString and any irregular lines, so refactoring to a line-based strategy (e.g., splitting by \n, ignoring blank/semicolon-only lines) will make your parsing robust. You also still use a for...of loop in src/convertToObject.js (around line 13), which conflicts with [CHECKLIST ITEM #5]; replace it with an iteration method such as reduce or forEach as required by the task. When you switch to reduce, avoid mutating an external accumulator object in violation of [CHECKLIST ITEM #1]; instead, build and return a new styles object in the reducer. Finally, rename the accumulator result (around line 9) to something more descriptive like stylesObject or stylesMap to comply with [CHECKLIST ITEM #2]. Once you address these issues, your solution should both pass the functional requirements and match the style constraints, and the overall structure you have now will adapt nicely to these improvements.
✨ 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
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override so you can keep progressing while you refine your approach. You did well by removing explicit loops, handling dashed property names, and keeping values with spaces intact, which shows good attention to the basic parsing requirements. However, your current use of sourceString.split(';') in src/convertToObject.js can still mishandle the provided multiline complexStylesString (for example, text-align: left !important; or lines that only contain ;), so convertToObject(complexStylesString) may not exactly match the expected object. To improve this, consider switching to a line-based approach: split by ' ', trim each line, ignore empty/;-only lines, and then parse key: value from each remaining line, explicitly stripping any trailing ; from the value. Also, instead of mutating an outer stylesObject inside forEach, try using reduce to build and return a new object step by step; this will better align with [CHECKLIST ITEM #1] about how iteration methods should be used. Overall, your progress is clear and you are close—tightening the parsing and using reduce will make your solution both more robust and fully compliant with the checklist.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| // write your code here | ||
| const stylesObject = {}; | ||
|
|
||
| const rules = sourceString.split(';'); |
There was a problem hiding this comment.
This approach of sourceString.split(';') is still the same as before and can mis-handle the multiline complexStylesString (e.g. semicolon-only lines, values like text-align: left !important;), which violates the requirement that convertToObject(complexStylesString) strictly equals the reference object; consider switching to a line-based strategy as mentioned in the previous review.
| const stylesObject = {}; | ||
|
|
||
| const rules = sourceString.split(';'); | ||
|
|
||
| rules.forEach((rule) => { | ||
| const cleanRule = rule.trim(); | ||
|
|
||
| if (!cleanRule) return; | ||
|
|
||
| const parts = cleanRule.split(':'); | ||
|
|
||
| if (parts.length < 2) return; | ||
|
|
||
| const key = parts[0].trim(); | ||
| const value = parts[1].trim(); | ||
|
|
||
| if (!key || !value) return; | ||
|
|
||
| stylesObject[key] = value; |
There was a problem hiding this comment.
Here you are mutating the external stylesObject inside forEach, which conflicts with checklist item #1: "Don't use iteration methods to modify an already existing array/object. (forEach is exception). These methods return a new array, so you should use it." Refactor to use something like reduce that builds and returns a new object instead of mutating an outer one.
| if (parts.length < 2) return; | ||
|
|
||
| const key = parts[0].trim(); | ||
| const value = parts[1].trim(); |
There was a problem hiding this comment.
The value is just parts[1].trim(), so trailing semicolons or additional content after : are not explicitly removed; this does not guarantee that values like '5px;' become '5px', which is required for strict equality with the expected object.
No description provided.