Skip to content

add solution#345

Open
6ORSH wants to merge 1 commit into
mate-academy:masterfrom
6ORSH:develop
Open

add solution#345
6ORSH wants to merge 1 commit into
mate-academy:masterfrom
6ORSH:develop

Conversation

@6ORSH

@6ORSH 6ORSH commented May 7, 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

Your implementation passes all requirements and the tests execute successfully. The test file correctly covers all three scenarios: function not returning anything, setting firstName from fullName, and handling users who already have firstName. One minor improvement worth noting: the third test uses expect(users).toEqual(users) which compares the array to itself—this assertion will always pass regardless of any mutations. Consider updating this to compare against a pre-defined expected array like [{firstName: 'Jack', fullName: 'Jack Black'}, {firstName: 'Mike', fullName: 'Mike Ross'}] to properly verify that existing firstName values remain unchanged. This is a test quality improvement rather than a functional issue, so your solution is approved.


✨ 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/restoreNames.test.js
Comment on lines +70 to +71

expect(users).toEqual(users);

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 assertion compares users to itself, which always passes regardless of mutations. Consider comparing against a pre-defined expected result array to ensure the original values are preserved when users already have firstName.

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