Skip to content

solution#4988

Open
bethrei wants to merge 4 commits into
mate-academy:masterfrom
bethrei:develop
Open

solution#4988
bethrei wants to merge 4 commits into
mate-academy:masterfrom
bethrei:develop

Conversation

@bethrei

@bethrei bethrei commented Jul 2, 2026

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because your solution, while functionally correct, still violates several explicit checklist rules that are considered critical for this task. The for loop in src/convertToObject.js (around the iteration over the style entries) breaks the requirement to avoid classic loops and instead use array iteration methods like map, filter, or reduce [CHECKLIST ITEM #5]. Inside your forEach, you are assigning back into styleEntries (e.g. styleEntries[index] = ...) and later using splice on the same array while iterating, which mutates an existing array instead of constructing a new one [CHECKLIST ITEM #1]; refactor this so that each transformation step returns a new array, for example using map followed by filter. Additionally, variable names like stylesArr and styleEntries are too generic and closely resemble discouraged names such as arr/result [CHECKLIST ITEM #2]; please rename them to something domain-specific like styleSegments, declarations, or stylePairs to better describe their content.

To move forward:

  • Replace the classic for loop with a chain of array methods that perform splitting, cleaning, and filtering of the style declarations without manual indexing.
  • Avoid mutating arrays during iteration; instead, build new arrays step by step (e.g. split → map to parse → filter to remove invalid entries → reduce to build the final object).
  • Rename the generic variables to clearer names that reflect what they store.

Your parsing logic for CSS styles, including handling of vendor prefixes and !important, is solid—once these structural and naming issues are addressed, your solution will fully meet the task 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/convertToObject.js Outdated
styleEntries[index] = stylesArr[index].split(':');
});

for (let entry = 0; entry < styleEntries.length; entry++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line starts a classic for loop, which violates checklist item #5: "Avoid using loops in this task, use iteration methods instead." Refactor this to use array methods like map, filter, or reduce instead of explicit indexing.

Comment thread src/convertToObject.js Outdated
Comment on lines +12 to +13
stylesArr.forEach((style, index) => {
styleEntries[index] = stylesArr[index].split(':');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you mutate styleEntries inside a forEach by assigning styleEntries[index] = .... 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." Consider returning a new array from map instead of manually filling styleEntries.

Comment thread src/convertToObject.js Outdated
*/
function convertToObject(sourceString) {
// write your code here
const stylesArr = sourceString.split(';');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using stylesArr as a name is very close to the discouraged arr naming and conflicts with checklist item #2: "Avoid variable namings like array, object arr, obj, result. It will decrease code readability." Prefer a more descriptive name, e.g. styleSegments or styleParts.

Comment thread src/convertToObject.js Outdated
function convertToObject(sourceString) {
// write your code here
const stylesArr = sourceString.split(';');
const styleEntries = [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, styleEntries is effectively an array of entries and serves as an intermediate "result" structure; this is close to the discouraged generic naming from checklist item #2. Consider a more specific name like rawPairs or stylePairs that describes its content.

Comment thread src/convertToObject.js Outdated
Comment on lines +24 to +25
if (!styleEntries[entry][key]) {
styleEntries.splice(entry, 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using splice here mutates the existing styleEntries array while iterating it. This falls under checklist item #1: "Don't use iteration methods to modify an already existing array/object" and also makes the loop logic more complex. Instead, consider using filter beforehand to remove empty keys, or build a new array of valid entries.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because there is still a critical, checklist-related issue that can break the expected behavior. In src/convertToObject.js, inside the forEach over stylePairs (around the transformation of each [key, value]), you mutate the existing entries and sometimes set the key to an empty string while leaving the pair in the array; later, Object.fromEntries(stylePairs) will still include those invalid entries, which can produce an empty-string property key and violates [CHECKLIST ITEM #1]. This also means malformed or empty declarations created by stray ; are not fully ignored as required, so the output object may not match the expected shape for the provided style strings.

To fix this, restructure the pipeline so that you never mutate stylePairs in place: for example, use stylesProperties.map(...) to return new normalized [key, value] pairs and then filter out pairs whose cleaned key is falsy before calling Object.fromEntries. Concretely, you want a chain like split → map(clean/trim) → filter(valid key/value) → Object.fromEntries, where each step returns a new array and invalid or empty declarations are removed entirely instead of being left as mutated entries. Once you avoid in-place mutation and ensure empty/malformed declarations are filtered out, your solution will satisfy both the checklist constraints and the functional requirements. You’ve already done good work removing classic loops and handling spaces, vendor prefixes, and !important correctly, so you’re very close—this refactor will complete the 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/convertToObject.js Outdated
Comment on lines +13 to +32
stylePairs.forEach((entry, index) => {
const key = 0;
const value = 1;

entry[key] = entry[key].replace(/\s+/g, ' ').trim();

if (!entry[key]) {
return;
}

let entryValue = entry[value];
const wordChars = [...entryValue.matchAll(/\S/g)];

entryValue = entryValue.slice(
wordChars[0].index,
wordChars.at(-1).index + 1,
);

entry[value] = entryValue;
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This transformation mutates the existing stylePairs array entries inside a forEach, and later you pass the whole mutated array to Object.fromEntries, including entries where entry[key] was empty and you returned early. This violates checklist item #1: "Don't use iteration methods to modify an already existing array/object" and also leaves invalid entries instead of filtering them out. Consider restructuring into a map that returns normalized [key, value] pairs and a filter that removes invalid ones before calling Object.fromEntries.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution as an educational override so you can keep progressing while you refine this pattern. You’ve done a good job structuring the pipeline with split(';')mapfilterObject.fromEntries, and you’re correctly treating each declaration as a [key, value] pair while preserving hyphenated and vendor-prefixed property names. The main remaining issue is in the filter step: since each pair is an array like [key, value], using pair.key on (for example) line where you filter will always be undefined, so !pair.key is always true and no invalid entries are removed. This means stray semicolons (like trailing ;) can produce an entry with an empty-string key (''), which violates the requirement that blank/stray declarations must not create properties and conflicts with [CHECKLIST ITEM #1] about correctly filtering after mapping. To fix this, keep returning [key, value] from map, but change your filter to check the first element of the tuple, e.g. normalizedStylePairs.filter(([key]) => key) so only truthy keys survive into Object.fromEntries. Overall, your structure and intent are solid; with that small but important filter correction, your solution will fully meet both the functional requirements and the checklist expectations.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/convertToObject.js
*/
function convertToObject(sourceString) {
// write your code here
const stylesProperties = sourceString.split(';');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall split(';')map structure is fine, but you must ensure that entries created from stray semicolons (which produce empty or whitespace-only strings) are ultimately removed, not allowed to pass through to the final object.

Comment thread src/convertToObject.js Outdated
return [key, value];
});

return Object.fromEntries(normalizedStylePairs.filter((pair) => !pair.key));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here pair is an array like [key, value], so pair.key is always undefined. This makes the predicate !pair.key always true, meaning nothing is filtered out; this violates the requirement to ignore empty/malformed declarations and contradicts the intended split → map(clean/trim) → filter(valid key/value) → Object.fromEntries pipeline from the previous review and checklist item #1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants