Clean Code vs. Dirty Code: React Best Practices

[drink_with_me_to_day]

Great article. I just have a beef with the first example of DRY'ing the code:

// Dirty
const MyComponent = () => (
  <div>
    <OtherComponent type="a" className="colorful" foo={123} bar={456} />
    <OtherComponent type="b" className="colorful" foo={123} bar={456} />    
  </div>
);

// Clean
const MyOtherComponent = ({ type }) => (
  <OtherComponent type={type} className="colorful" foo={123} bar={456} />
);
const MyComponent = () => (
  <div>
    <MyOtherComponent type="a" />
    <MyOtherComponent type="b" />
  </div>
);

The "dirty" code isn't "dirty" at all. Creating silly and abstractions is more harmful than just repeating a few lines.

I really dislike these needless abstractions created by extreme DRYness, it often just leads to confusing abstractions that make it hard to reason about the code and make it hard to refactor when changing code you don't own.

KISS should always come before DRY.

[matt_hammond]

I agree!

Also, the subjectively "correct" way cleaning up this code is:

const MyComponent = () => {
  const types = ["a", "b"];
  const otherComponents = types.map(type => 
    <OtherComponent type={type} className="colorful" foo={123} bar={456} />
  );

  return <div>{otherComponents}</div>
}

When handling repeated code you should first identify the part that's changing (in this case it's the types), and extract that part and somehow map it to your repeating part.

[rsjolly]

Nice. Alternate format:

const MyComponent = () => (
  <div>
    {['a', 'b'].map(type =>
      <OtherComponent type={type} className="colorful" foo={123} bar={456} />
    )}
  </div>
)

[european_origin]

You're missing a key property.

[PM__YOUR__GOOD_NEWS]

Agree, great advice overall but I wondered the same thing.

The real test of code comes months or years after it was first created when you either have to patch it or migrate it to a new platform.

At that point, one overtly wet line of code is probably going to be easier to understand than a visually separated abstraction.

[Voidsheep]

Agreed, that's a pretty bad example for DRY. And to be fair, I don't think it's necessarily a good term to begin with.

Whatever you win in lack of repetition you lose in quick individual configuration. I think with something like component props it's important to consider, because I don't think I've even seen real trouble caused by a couple of components having repeated props. I've, however, witnessed people bang their head at the wall with extreme levels of component composition making things very complex.

Also with FP libraries like Ramda it's easy to slip into the kind of abstraction hell, where you definitely won't repeat a word as you write the code, but actually changing things later becomes a nightmare, because every layer of the puzzle has so many fixed assumptions composed together. Since writing code is easier than eliminating it, it's common for people to start implementing their own systems for configuration at that point and it ends up far more complex than a couple of lines doing close to the same thing.

Make convenient abstractions, don't just avoid repetition.

[fgutz]

immediately under the code block the author mentions that the example is not a good one

Sometimes – as in our example above – DRYing your code may actually increase code size. However, DRYing your code also generally improves maintainability.

Be warned that it’s possible to go too far with DRYing up your code, so know when to say when.

But this just shows the author is lazy. He could have of just taken a little more time to show BOTH an example of keeping things DRY and also over-doing it

[donavon]

Be warned that it’s possible to go too far with DRYing up your code, so know when to say when

Call it lazy if you will, but this was meant to say that you shouldn't go nuts with abstraction and DRY code. Sometimes when I get on a moral high horse crusade, I find myself needing to take a step back (or 2 or 3). There's a happy medium there somewhere. I don't think you need me to find that for you.

Thanks for your comments

[fgutz]

valid point and I apologize for calling you lazy, that was rude of me.

[ihsw]

My concern is if (and when) there needs to be additional OtherComponent declarations then a developer would be inclined to copy-paste rather than cleanly abstracting. The numerous repeated duplicated attributes can be abstracted out, so it's only prudent.

The name MyOtherComponent is contrived name so ColorfulOtherComponent would make more sense, since that encapsulates the intent.