0

I have a pull request and this pr is being reviewed by co-worker.

Context

I'm using React and a specific component has ternary operator within component like below.

const ExampleComponent = (): React.ReactElement => {
  return (
    <div onClick={condition ? handlerA : handlerB}>
      <span>{condition ? textA : textB}</span>
      {condition ? <ChildA /> : <ChildB />}
    </div>
  );
}

In above code, my co-worker suggested like below.

const ComponentA = (): React.ReactElement => {
  return (
    <div onClick={handlerA}>
      <span>{textA}</span>
      <ChildA />
    </div>
  );
}

const ComponentB = (): React.ReactElement => {
  return (
    <div onClick={handlerB}>
      <span>{textB}</span>
      <ChildB />
    </div>
  );
} 

const ExampleComponent = (): React.ReactElement => {
  return (
    <>{condition ? <Component A /> : <Component B />}</>
  );
}

My opinion : can reduce duplication of tag, classname, text.
co-worker opinion : readability is better.

I thought deeply about these talking, but couldn't conclude which is the right way to make code clean.

Question

How can I decide which structure is better? Thanks for your reading!

1
  • I would choose either the first or second approach but by creating a base component that receives the text, onClick and uses children (if it makes sense in this context). That way you reduce code duplication.
    – YohaiM
    Jun 21 at 4:31
1

As already pointed out, your implementation will become a nightmare very quickly once more options come in or when certain options need to differ slightly.

However the duplication of structure is a valid concern. With your co-workers implementation, changing this structure or its details will see you changing every component.

You could create a more resuable component by pushing the conditions out and into the parent:

const Component = (): React.ReactElement => {
  return (
    <div onClick={onClick}>
      <span>{text}</span>
      {children}
    </div>
  );
}

const ExampleComponent = (): React.ReactElement => {
  return (
    {condition ?
      <Component text={A} onClick={AHandler}>
        <ChildA />
      </Component> :
      <Component text={B} onClick={BHandler}>
        <ChildB />
      </Component>
  );
}
0

I would strongly prefer the approach with the ternary outside the components. This allows for each child component to be independent and simpler, and also allows for easy accommodation when the inevitable 3rd child component option appears.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.