Massimiliano Mirra

Notes















Date:
Tags: code · collaboration
Status: draft

Comment the code that is not there

I recently wrote code similar to this:

const App = (props) => {
  return (
    <ThemeProvider theme={theme}>
      <PageContent {...props} />
    </ThemeProvider>
  )
}

const PageContent = ({ Component, pageProps }) => {
  const getLayout = Component.getLayout ?? ((page) => page)
  return getLayout(<Component {...pageProps} />)
}

If it were my first time reading it, I’d think that the PageContent component is a pointless, even harmful addition: it’s only used once; it doesn’t express any remarkable idea; it forces the reader to store yet another name in an already crowded working memory.

I’d be strongly tempted to refactor it, and since it’s a non-functional change, I might do it as part of a larger change, heeding the boy scout rule:

const App = (props) => {
  const getLayout = Component.getLayout ?? ((page) => page)

  return (
    <ThemeProvider theme={theme}>
      {getLayout(<Component {...pageProps} />)}
    </ThemeProvider>
  )
}

I might even skip refreshing the page, because, again, it’s just a change in form.

Or is it?

I’d later find out that the background no longer respects the dark mode setting, while every other component does; after trying for a long while to debug the larger functional change without getting anywhere, I would trace the problem to the code above.

If you’re trying to figure out how the refactoring attemept could cause that, good luck. Although you might spot something if you’re well versed in React and you squint hard and you’ve encountered the circumstance before, it’s really hard, because the code you need to know the most to figure out what’s going on is nowhere in sight, it’s instead in the source for the page component itself, in a file far away:

export const getLayout = (page) => (
  <Layout bgImage={useColorModeValue(backgroundLight, backgroundDark)}>
    {page}
  <Layout>
)

Even so, still not obvious…

Imagine, however, that while writing the original code, I had placed this comment inside the PageContent component:

const App = (props) => (
  return (
    <ThemeProvider theme={theme}>
      <PageContent {...props} />
    </ThemeProvider>
  )
)

const PageContent = ({ Component, pageProps }) => {
  // Why not just have all this in <App>? Because getLayout uses the
  // color mode hook, which in turn must be called from under
  // <ThemeProvider> to work

  const getLayout = Component.getLayout || ((page) => page)
  return getLayout(<Component {...pageProps} />)
}

How likely is the reader now to jump into refactoring?

It’s second nature for programmers, while they’re writing code, to imagine it playing out at run time and continuously ask: “what’s likely to go wrong here? what could cause this to break? how can I prevent or handle it?” Users may provide invalid input, network connections might go down, the environment might not support the cabability, … Situations handled with schemas, timeouts, discovery, …

It pays to adopt the same mentality and, while you’re writing code, imagine it playing out at read time and ask: “what’s likely to go wrong here? what could cause this to be misunderstood? how can I prevent or handle it?” Commenting code that’s not there — explaining the outside-of-source reasons why the code is the way it is, instead of a more “logical”, “elegant”, or “obvious” form — is a relatively cheap and absolutely effective way to prevent read time bugs.

Join the newsletter

© 2020-2023 Massimiliano Mirra