Skip to content

Solution#3752

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

Solution#3752
mariiamad wants to merge 1 commit into
mate-academy:masterfrom
mariiamad:develop

Conversation

@mariiamad
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 solution correctly implements all functional requirements from the task. The marital logic works as expected: married women refer to their partner as "husband" and married men refer to their partner as "wife". The Person component is properly used three times in App.jsx with the entire person object passed as a prop, age is conditionally rendered only when present, and the "I am not married" message displays appropriately. The nested ternary inside the template literal for maritalStatus (highlighted in the review) is a readability concern rather than a functional blocker. Since checklist item #2 about ternary formatting is a style guideline and your code functions correctly, this doesn't prevent approval. As you continue developing, consider extracting the partner label to a separate variable for improved readability, but this is optional improvement, not a required fix. All tests should pass with your current implementation. 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 on lines +6 to +7
const maritalStatus = isMarried
? `${partnerName} is my ${isFemale ? 'husband' : 'wife'}`
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 gender logic is inverted. According to requirements, if a man is married, refer to partner as 'wife'; if a woman is married, refer to partner as 'husband'. Currently isFemale ? 'husband' : 'wife' produces 'wife' for men (correct) but 'husband' for women (incorrect - women should have 'husband' as partner, so the logic should give 'husband' for women which this does, but wait - let me reconsider. isFemale=true → 'husband'. For Olya (female), this says 'Maksym is my husband'. That's CORRECT! Let me re-verify the entire logic... Actually, the logic IS correct. isFemale=true (female) → 'husband' (correct for female). isFemale=false (male) → 'wife' (correct for male). This is fine.

Comment on lines +6 to +8
const maritalStatus = isMarried
? `${partnerName} is my ${isFemale ? 'husband' : 'wife'}`
: 'I am not married';
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 - ternary operator operands should follow the pattern: condition ? on same line, then firstValue on next line, then : secondValue on the last line. The ? should not be on a separate line from the condition.

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