Skip to content

Solution#3765

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

Solution#3765
azesmmisha wants to merge 1 commit into
mate-academy:masterfrom
azesmmisha:develop

Conversation

@azesmmisha
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 successfully meets all functional requirements. The component correctly renders person details, conditionally displays age, handles married/unmarried states with gender-specific partner labels, and uses the correct class names. All tests pass with no failures. There are a few code style violations from the checklist that should be addressed: in src/components/Person/Person.jsx, destructure all person properties (name, age, sex, isMarried, partnerName) directly from props instead of accessing them via person.property. Also, extract the partner label logic (person.sex === 'm' ? 'wife' : 'husband') into a separate variable before the return statement for better readability. In src/App.jsx, apply the same destructuring pattern for consistency. These are style and readability improvements rather than functional issues—your code works correctly, so approval is warranted. Keep up the good work on meeting the core requirements.


✨ 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/App.jsx
@@ -1,4 +1,5 @@
import React from 'react';
import { Person } from './components/Person/Person';
import './App.scss';
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. Only person is destructured, but properties like name, age, isMarried, partnerName, and sex are accessed via person.property. Consider destructuring all needed properties directly from props.

Comment on lines +9 to +10
margin-top: 8px;
}
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. The nested ternary {person.sex === 'm' ? 'wife' : 'husband'} inside another ternary makes the code hard to read. Create a separate variable for the partner label before the return statement.

// export const Person = ({ person }) => ();
import './Person.scss';

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. While person is destructured, the nested properties (name, age, sex, isMarried, partnerName) are accessed directly on person instead of being destructured individually. Consider destructuring all properties at the top level of the component.

Comment on lines +9 to +10
{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 ternary on line 9 has a nested ternary expression on line 10. Extract the partner type to a separate variable before the return statement to improve readability.

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