Skip to content

Solution#3726

Open
cassiaqueiroz wants to merge 2 commits into
mate-academy:masterfrom
cassiaqueiroz:develop
Open

Solution#3726
cassiaqueiroz wants to merge 2 commits into
mate-academy:masterfrom
cassiaqueiroz:develop

Conversation

@cassiaqueiroz
Copy link
Copy Markdown

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

Your implementation correctly fulfills all core functional requirements - the Person component properly displays names, conditionally shows age, and handles gender-specific partner terminology exactly as specified. All test cases pass.

However, there are 3 checklist style violations that should be addressed:

  1. [CHECKLIST ITEM #1] The person prop is destructured in the function signature, but you then access properties via person.name, person.age, etc. instead of destructuring those properties directly. Add const { name, age, sex, isMarried, partnerName } = person; at the beginning of your component.

  2. [CHECKLIST ITEM #2] The ternary operator in your JSX has operands on the same line. Move each operand to a separate line for better readability.

  3. [CHECKLIST ITEM #3] The isMarried condition appears inline in your JSX ternary expression. Extract it to a variable like const partnerText = isMarried ? ... : ...; before the return statement.

Additionally, remove the commented-out code on line 2: // export const Person = ({ person }) => ();

These are style guideline violations - the functionality works perfectly, so your solution is approved. Consider fixing these issues to improve code quality.


✨ 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/components/Person/Person.jsx Outdated
@@ -1 +1,16 @@
// export const Person = ({ person }) => ();
export const Person = ({ person }) => (
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 violates checklist item #1: 'Use destructuring for getting access to values of props object'. The component receives person as a destructured prop but then accesses all properties via person.propertyName instead of destructuring the person object's properties directly.

Comment thread src/components/Person/Person.jsx Outdated
Comment on lines +8 to +9
{person.isMarried
? `${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}`
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 violates checklist item #2: 'Format ternary operator operands correctly - move each operand to the separate line'. The ternary operands should be on separate lines for better readability.

Comment thread src/components/Person/Person.jsx Outdated
{person.age && <p className="Person__age">I am {person.age}</p>}
<p className="Person__partner">
{person.isMarried
? `${person.partnerName} is my ${person.sex === 'm' ? 'wife' : 'husband'}`
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 violates checklist item #3: 'Avoid putting several cases to conditional rendering. Create separate variable for the condition'. The nested ternary person.sex === 'm' ? 'wife' : 'husband' should be extracted to a separate variable.

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