add task solution #1616
add task solution #1616
Conversation
src/components/Person/Person.jsx
Outdated
| <h2 className="Person__name">{`My name is ${person.name}`}</h2> | ||
|
|
||
| <> | ||
| {person.age && ( |
There was a problem hiding this comment.
Let's get needed properties from person object using destructuring before the return statement
src/components/Person/Person.jsx
Outdated
| {person.isMarried && person.sex === 'm' && ( | ||
| <>{`${person.partnerName} is my wife`}</> | ||
| )} | ||
| {person.isMarried && person.sex === 'f' && ( | ||
| <>{`${person.partnerName} is my husband`}</> | ||
| )} | ||
| {!person.isMarried && <>I am not married</>} |
There was a problem hiding this comment.
You've repeated some part of code
Will be better to define the info about the partner initially, should it be wife or husband
And then use this data while preparing the content
src/components/Person/Person.jsx
Outdated
| const WOMAN_GENDER = 'f'; | ||
| const MAN_GENDER = 'm'; |
There was a problem hiding this comment.
Such constants are better defined outside the component.
src/components/Person/Person.jsx
Outdated
| <section className="Person"> | ||
| <h2 className="Person__name">{`My name is ${name}`}</h2> | ||
|
|
||
| <> |
There was a problem hiding this comment.
Do we really need to wrap everything in a fragment here?
src/components/Person/Person.jsx
Outdated
|
|
||
| function getMarriedInfo() { | ||
| if (sex === MAN_GENDER && isMarried) { | ||
| return <>{`${partnerName} is my wife`}</>; |
There was a problem hiding this comment.
Do we really need to wrap everything in a fragment here?
src/components/Person/Person.jsx
Outdated
| } | ||
|
|
||
| if (sex === WOMAN_GENDER && isMarried) { | ||
| return <>{`${partnerName} is my husband`}</>; |
There was a problem hiding this comment.
Do we really need to wrap everything in a fragment here?
src/components/Person/Person.jsx
Outdated
| return <>{`${partnerName} is my husband`}</>; | ||
| } | ||
|
|
||
| return <>I am not married</>; |
There was a problem hiding this comment.
Do we really need to wrap everything in a fragment here?
src/components/Person/Person.jsx
Outdated
| partnerName, | ||
| } = person; | ||
|
|
||
| function getMarriedInfo() { |
There was a problem hiding this comment.
Such a function can also be taken outside the component definition.
src/components/Person/Person.jsx
Outdated
|
|
||
| const MAN_GENDER = 'm'; | ||
|
|
||
| function getMarriedInfo(status, gender, companion) { |
There was a problem hiding this comment.
It is worth choosing more meaningful names for arguments so that they explain what is happening in the code.
| function getMarriedInfo(status, gender, companion) { | |
| function getMarriedInfo(isMarried, gender, partnerName) { |
| {age && ( | ||
| <p className="Person__age"> | ||
| {`I am ${age}`} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
If the "age" is 0 we will get the 0 shown
DEMO LINK