August 28, 2020

Multiple Boolean Props are a Code Smell

edit

You learn a lot working on a large, complicated codebase for an extended period of time. It’s the kind of environment that exposes you to particular patterns often enough to develop thoughts and opinions around them. I want to share a quick opinion I have regarding a particular pattern here: multiple boolean props or state in a component is probably a code smell.

Imagine a CommonButton component for an application. This CommonButton has a default way of rendering:

<CommonButton>Click Me!</CommonButton>

Now, imagine our app needs to change the background of the Button depending on the information that button needs to convey. Let’s have a few different variations: a primary, warning, and danger styles. We could do this by passing our CommonButton a prop of that name.

<CommonButton primary>Click Me!</CommonButton>
<CommonButton warning>Click Me!</CommonButton>
<CommonButton danger>Click Me!</CommonButton>

What should this code look like? Maybe something like this:

function CommonButton({
  children,
  onClick = () => {},
  primary,
  warning,
  danger,
  type = 'button',
}) {
  let backgroundColor = '#33a1cc'
  if (primary) backgroundColor = '#17b890'
  if (warning) backgroundColor = '#ffd166'
  if (danger) backgroundColor = '#f0544f'

  return (
    <button onClick={onClick} type={type} style={{ backgroundColor }}>
      {children}
    </button>
  )
}

And then when we render our buttons from before, we would get:

That looks great, but there are some serious problems with this approach that make it rather smelly. The problem that bothers my coding olfactory senses the most is the existence of impossible states.

What should happen if the consumer does the following?

<CommonButton danger primary warning>
  Click Me!
</CommonButton>

Should it render as green, yellow, or red? Let’s find out.

If you put more than one of these boolean props in place, what happens? You see a leaking of the implementation details! Whatever way the component creator has decided to reconcile these impossible states is exposed. In this case, I wrote it so that danger was the last state checked that updates the background color. Therefore, the danger prop takes priority over all the other props, regardless of the order they are passed in.

Now, admittedly, this is not how we intend our component to be used. “The consumer should use the component correctly!” you might yell. True. They probably should, but we shouldn’t write code that depends on others to just not make mistakes. Especially when we can programmatically defend against the problem in a simple way.

We could possibly stop the abuse of our CommonButton through an error/warning or even an ESLint rule, but there’s a much easier thing that we can do. Get rid of the multiple boolean props.

Whenever we have multiple boolean props, especially ones related to the same concern, these should really be expressed as a single prop with an enumerated set of strings as the possible values. Let’s make a BetterButton that follows this pattern.

const BUTTON_BGS = {
  default: '#33a1cc',
  primary: '#17b890',
  warning: '#ffd166',
  danger: '#f0544f',
}

function BetterButton({
  children,
  onClick = () => {},
  type = 'button',
  variant = 'default',
}) {
  const backgroundColor = BUTTON_BGS[variant] || BUTTON_BGS.default

  return (
    <button onClick={onClick} style={{ backgroundColor }} type={type}>
      {children}
    </button>
  )
}

This button not only moves our props into a single concern, but allows us to:

  • Move our styles into an enum
  • Handle incorrect variants gracefully
  • Make impossible states impossible
  • Makes later extension as simple as adding a variant

Now, our buttons look like this:

<BetterButton variant="primary">Click Me!</BetterButton>
<BetterButton variant="warning">Click Me!</BetterButton>
<BetterButton variant="danger">Click Me!</BetterButton>

Even better, when you try and give it some garbage as a variant, it doesn’t leak our implementation details, it just uses the default styling.

<BetterButton variant="primary warning danger">Click Me!</BetterButton>
<BetterButton variant="garbage">Click Me!</BetterButton>

This also applies to situations where you have multiple booleans around different concerns coming in. It’s likely that what you really have are a number of finite “states” your component can be in, and it would be better to derive that state and pass it in as a prop, rather than muddy your code with conditional checks for your many booleans.

Again, this is just my opinion. I don’t have any way of definitively proving that this approach is better, but I’ve found it to be useful for me in my career. Hope you also find this information useful.


Liked the post?
Give the author a dopamine boost with a few "beard strokes". Click the beard up to 50 times to show your appreciation.
Want to read more?
Need help with your software problems?

My team and I are ready to help you. Hire Agathist to build your next great project or to improve one of your existing ones.

Get in touch
Kyle Shevlin's face, which is mostly a beard with eyes

Kyle Shevlin is the founder & lead software engineer of Agathist, a software development firm with a mission to build good software with good people.

Agathist
Good software by good people.
Visit https://agath.ist to learn more
Sign up for my newsletter
Let's chat some more about TypeScript, React, and frontend web development. Unsubscribe at any time.
Logo for Introduction to State Machines and XState
Introduction to State Machines and XState
Check out my courses!
If you enjoy my posts, you might enjoy my courses, too. Click the button to view the course or go to Courses for more information.