const contentCards: any[] = sortBy(
[
{
title: 'title1',
field: //array of objects,
},
{ title: 'title2',
field: //array of objects,
},
{
title: 'title3',
hidden: true,
field: //array of objects,
},
{
title: 'title4',
hidden: true,
field: //array of objects,
},
{
title: 'title5',
field: //array of objects,
},
]
);
const contentFields = React.useMemo(() =>
contentCards.filter(card => {
const values = card.fields.filter(
field => getFieldValue(field) !== null
);
return (
!isEmpty(values) && (isSSH ? !card.hidden : true)
);
}),
[getFieldValue, isSSH]
);
return (
{!isActive && (
<FormField label="Content" >
{() => (
<>
{contentFields.map(card =>
//this renders each card
)}
</>
)}
</FormField>
)}
);
When isSSH
and !isActive
, i want to show all cards except the one with title3 and title 4.
So below is how it should work if
!isSSH show all cards
isSSH and isActive show all cards except title3 and title4
isSSH and !isActive dont show any cards.
the above contentFields method actually returns all cards if !isSSH and if isSSH returns all except title3 and title4.
this line in code
return (
!isEmpty(values) && (isSSH ? !card.hidden : true)
);
does the filtering of cards based on hidden value.
now to fix this as i want i have changed contentFields method like below,
const contentFields = React.useMemo(() =>
contentCards.filter(card => {
const values = card.fields.filter(
field => getFieldValue(field) !== null
);
if (!isEmpty(values)) {
if (!isSSH) return true;
if (isSSH && !isActive) return !card.hidden;
if (isSSH && isActive) return false;
}
}),
[getFieldValue, isSSH, isActive]
);
the above code works as i want. but could some one help me make this if part of code cleaner. better way of handling it. could someone please help me with this. thanks.
CodePudding user response:
Looks close!
const contentFields = React.useMemo(() =>
contentCards.filter(card => {
const values = card.fields.filter(
field => getFieldValue(field) !== null
);
// You can save some nesting by returning
// earlier on the `isEmpty` condition.
if (isEmpty(values)) {
return false
}
if (!isSSH) return true;
// as you alreay checked for !isSSH, you
// won't need to do that again below
if (!isActive) return !card.hidden;
// As you've already covered !isSSH and !isActive,
// the last if isn't needed so you only need a return
return false;
}),
[getFieldValue, isSSH, isActive]
);
CodePudding user response:
My suggestion is I can see redundancy in your isEmpty function. You can re-write your function like this:
if (!isEmpty(values)) { if (!isSSH) return true; if (!isActive) return !card.hidden; // Executed if(isSSH & !isActive) return false; // Only executed if(isSSH & isActive) }
See, when you are adding a return inside an if(!something) check, then the further code execution will only happen if it will opposite scenario (like - if(something)). So any check for if(something) in further code is redundant.
Edit: I also do not recommend the ternary operator in this case because I believe in the one-line -> one-action philosophy which makes code easier to read and understand during the quick vertical scan.
Edit 2: You will need to return true/false (as per your requirement) outside of the if(!isEmpty(){...}) case. As a practice, you should have consistent returns.
CodePudding user response:
This looks clean, ternary will look messed up. Here's the ternary verison.
!isEmpty(values) && (isSSH ? (isActive ? false : !card.hidden) : true)