resolving task#4983
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because, although the core parsing logic works, several explicit requirements are not yet met. On src/convertToObject.js, your output variable is named result (see near the top of the function), which violates the naming guideline to avoid generic names like result [CHECKLIST ITEM #2]; please rename it to something domain-specific like stylesObject. The loop over declarations (the for...of around the middle of the file) conflicts with the requirement to use array iteration methods instead of explicit loops [CHECKLIST ITEM #5]; refactor this to use filter and reduce or similar. Additionally, you currently build the object by mutating an external accumulator; the checklist suggests using reduce to build and return a new object rather than incrementally mutating an existing one, so rework that part accordingly. Finally, the task description states that console.log(result === { ... }) must output true, but your function returns a new object each time, so === will be `false because it compares references; you need to adjust your approach so the equality requirement is satisfied or clearly handled in line with the instructions. The good news is that your trimming, handling of empty declarations, and preservation of property/value pairs are all correct—once you align the iteration style, naming, and equality behavior with the checklist, your solution will be in great shape.
✨ 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
| */ | ||
| function convertToObject(sourceString) { | ||
| // write your code here | ||
| const result = {}; |
There was a problem hiding this comment.
This violates checklist item #2: "Avoid variable namings like array , object arr, obj, result. It will decrease code readability" because the variable is named result. Consider using a more descriptive name like stylesObject or similar.
| const result = {}; | ||
| const declarations = sourceString.split(';'); | ||
|
|
||
| for (const declaration of declarations) { |
There was a problem hiding this comment.
This violates checklist item #5: "Avoid using loops in this task, use iteration methods instead." The for...of loop over declarations should be replaced with iteration methods such as filter and reduce on the declarations array.
| const key = rawKey.trim(); | ||
| const value = rawValue.trim(); | ||
|
|
||
| result[key] = value; |
There was a problem hiding this comment.
This violates checklist item #1: "Don't use iteration methods to modify an already existing array/object. (forEach is exception)." Even though you're using a loop here rather than an iteration method, you are incrementally mutating the result object; to align with the intent of the checklist, consider using reduce to build and return a new object without mutating an external accumulator.
| result[key] = value; | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
According to the description, console.log(result === { ... }) must output true, but in JavaScript === compares object references, not structural equality. This implementation returns a freshly created object, so result === { ... } will always be false. You need to address this requirement, either by matching the expected identity semantics or clarifying/adjusting the approach to equality in line with the task constraints.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still critical mismatches with both the task requirements and the checklist constraints. The implementation of convertToObject in src/convertToObject.js currently uses forEach to mutate an external stylesObject object, which directly violates the requirement to use iteration methods to build and return new data rather than modify an existing object [CHECKLIST ITEM #1]. Instead, you should refactor this logic to use something like declarations.reduce(...) that constructs and returns the final object without mutating an external accumulator.
A second critical issue is that the task explicitly states that console.log(result === { ... }) should output true, but your function always returns a fresh object, so strict equality with a literal will necessarily be false; you need to adjust your implementation or how the reference object is created so that the equality comparison can actually succeed. Additionally, declarations of the form "property:" currently produce a key with an undefined value, which is outside the target behavior and should be guarded against (e.g., skip declarations where there is no non-empty value after the colon). As a minor improvement, the return {}; inside the forEach callback is misleading because it does nothing to the outer function; removing it or restructuring the logic will make the code clearer.
Once you refactor to use reduce (or another appropriate iteration method) to build and return the styles object, ensure the strict equality requirement is met, and avoid undefined values for malformed declarations, your solution will be in line with the task and checklist. You’ve done a good job handling trimming and ignoring completely empty declarations, so focus on these structural issues to finish the exercise strong.
✨ 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
| */ | ||
| function convertToObject(sourceString) { | ||
| // write your code here | ||
| const stylesObject = {}; |
There was a problem hiding this comment.
This violates 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. Here stylesObject is declared and then mutated inside forEach. Consider building and returning the object via reduce instead of mutating an external accumulator.
| const stylesObject = {}; | ||
| const declarations = sourceString.split(';'); | ||
|
|
||
| declarations.forEach((declaration) => { |
There was a problem hiding this comment.
Within this forEach, you are using an iteration method but returning nothing from it and mutating stylesObject. According to checklist item #1, you should use iteration methods to produce new arrays/objects, not to modify an existing one. A reduce over declarations that returns the final object would better align with the requirement.
| if (declaration.trim() === '') { | ||
| return {}; |
There was a problem hiding this comment.
The return {}; inside forEach does not affect the outer function and creates a new object that is immediately discarded. While it correctly skips empty declarations, it is unnecessary and may be misleading. More importantly, the overall approach of using forEach to mutate stylesObject conflicts with checklist item #1’s guidance on iteration methods.
| stylesObject[key] = value; | ||
| }); | ||
|
|
||
| return stylesObject; |
There was a problem hiding this comment.
Given the task description, convertToObject currently returns a new object on each call, so console.log(result === { ... }) will be false because === compares object references. The description explicitly says this comparison // outputs true;, so you need to adjust your implementation or how the reference object is created so that strict equality can actually be true.
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 the last detail. You’ve done a strong job with the core parsing logic in src/convertToObject.js: using split and reduce to build a new object, avoiding loops, and correctly skipping empty or malformed declarations all match the requirements ([CHECKLIST ITEM #1] and [CHECKLIST ITEM #5]). One remaining conceptual issue is the strict equality requirement: console.log(result === { ... }) can never be true if convertToObject always returns a freshly created object, because inline object literals create new references each time. To fix this, you’d need to either ensure result and the comparison share the same object instance (for example, by comparing to a previously stored object) or change the check to a deep equality test rather than ===.
Even with that mismatch, the structure and readability of your code are solid, your variable naming avoids the banned patterns, and you are correctly returning a new object rather than mutating an external one. Keep focusing on understanding reference vs. value equality in JavaScript—that’s the main learning opportunity here—and overall you’re clearly on the right track.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.