¡Estamos en directo en Twitch!

¡Entra y participa!

Imagen de la etiqueta best-practice

Single Line Responsibility, one line does one thing

· 8 minutos de lectura · ¿Una errata? Edita el artículo

It is said that is easier writing understandable code for computers than for humans. And who doesn’t believe that? No matter the name of your variables, code styling, or weird decisions that you could make that, if it compiles, the computer is going to do what it has to do. And that’s cool as we are free to throw a bunch of spaghetti to our beloved machines and get the work done.

That said, the main problem is, generally speaking, our code should be read as well by humans. Colleagues, teammates, managers, clients… truth is our code should be understandable by machines but readable by humans.

Some years ago, I asked a teammate why he used single letters as names for variables. He said: “So I will be needed for understanding this code”.

There’re a lot of patterns and architectures, depending on the programming language, that could help you to write better code. Still, there’s a simple yet powerful principle that could help you a lot to achieve this. I call it: Single Line Responsibility™. And it has a pretty simple rule: use each line to do a single thing.

Let’s see some real production code where the Single Line Responsibility is NOT being used:

/* examples of doing MORE than one thing in one line */
/* (spoiler: you're gonna need horizontal scroll) */

// get an element, add an event and pass the callback with some calculated params
document.querySelector('.js-push-button').addEventListener('click', function () { pushEnabled && pushMessage(JSON.stringify(messages))})

// check condition, concatenate two dots with a substring and assign to a variable
if (value.length > 13) value = value.substring(0,12) + '..';

// let's push to grid a created path with a bunch of params no body understand!
grid.push(paper.path("M0 " + i + " L" + getPaperWidth(i) + " " + i, false))

// feel like there you are a god  
e.innerHTML = t._getDateTime(new Date(), ed.getParam("template_mdate_format", ed.getLang("template.mdate_format")))

Does it feel nasty? Oh my, it should feel nasty and that’s good. That means that you’re willing to know more about the Single Line Responsibility advice.

Single Line Responsibility is a rule that enforces you to try to do only one thing in each line.

Using Single Line Responsibility, for good

While the rule is pretty simple, how to achieve it could be highly subjective. There’s plenty of ways to achieve the desired effect. A wrong way to do that could lead to a worse readable code so be careful and try with different things to approach the problem.

In order to help you understand the principle, let’s check some examples:

Avoid creating objects and/or params by calling methods and using conditionals:

// ❌ we're creating a searchUrlValueObject but for each key we
// have some special logic to create its values
return SearchFactory.searchUrlValueObject({
  featureLiteral:
    featureIds && featureIds.length === 1
      ? this._findFeatureByValue(featureIds[0])
      : undefined,
  // is the template string really necessary? hard to know!
  urlPattern: `${this._config('routing')[isMap ? 'map' : 'search']}`
})

// ✅ refactored following the Single Line Responsibility™
// create a const for the featureLiteral
const featureLiteral = featureIds && featureIds.length === 1
  ? this._findFeatureByValue(featureIds[0])
  : undefined
// extract the ternary in a const to use later
const routingType = isMap ? 'map' : 'search'
// remove the unnecesary template string and use the routingType
const urlPattern = this._config('routing')[routingType]
// use short-hand for key-value object and return the searchUrlValueObject
return SearchFactory.searchUrlValueObject({featureLiteral, urlPattern})

Template strings are powerful just don’t bloat them with logic.

// ❌ logic in the template string and using it as a parameter  
writeFile(
    COMPONENT_PACKAGE_JSON_FILE,
    `{
  "name": "${packageName}",
  "version": "1.0.0",
  "description": "",
  "main": "lib/index.js",
  ${
    repository.url
      ? `
  "repository": {
    "type": "${repository.type}",
    "url": "${repository.url}"
  },`
      : ''
  }${
      homepage
        ? `
    "homepage": "${homepage.replace('/master', `/master${COMPONENT_DIR}`)}",`
        : ''
    }
  "keywords": [],
  "author": "",
  "license": "MIT"
}`
    
// ✅ refactored following the Single Line Responsibility™
const createHomePageSection = homepage => {
  if (!homepage) return ''

  const homepageValue = homepage.replace('/master', `/master${COMPONENT_DIR}`)
  return `"homepage": "${homepageValue}",`
}

const createRepositorySection = ({url, type}) => {
  if (!url) return ''

  return `"repository": {
    "type": "${type}",
    "url": "${url}"
  },`
}

const fileContent = `{
  "name": "${packageName}",
  "version": "1.0.0",
  "description": "",
  "main": "lib/index.js",
  ${createRepositorySection(repository)}
  ${createHomePageSection(homepage)}
  "keywords": [],
  "author": "",
  "license": "MIT"
}`

writeFile(COMPONENT_PACKAGE_JSON_FILE, fileContent)

Chaining could be good, just use it separating per line:

// ❌ reading could be a bit difficult
listOfNumbers.filter(n => n % 2 === 0).map(n => n * n).some(n => n > 30)

// ✅ separate each line, linter could help you with this!
listOfNumbers
  .filter(n => n % 2 === 0)
  .map(n => n * n)
  .some(n => n > 30)

Destructuring is nice. Just, don’t make it ugly:

// ❌ I'm scared
const { result: { realEstates: { count = 0 } = {} } = { realEstates: {} } } = props

// ✅ not the best but I'll sleep at night
const { results = {} } = props
const { realEstates = {} } = results
const { count = 0 } = realEstates

It works for React components as well!

// ❌ it's like the callback hell! 🔥
render() {
  const {showCardSubscription} = this.state
  const {appName, i18n} = this.props

  return (
    <div>
      {this._hasFailed(this.props.errorMessage) && this._isRegistered() && (
        <SuiCardSubscription
          checkboxLabel={
            <span
              className="mt-CardSubscription-checkboxLabel"
              dangerouslySetInnerHTML={{
                __html: i18n.t(
                  `CARD_SUBSCRIPTION_${appName}_CHECKBOX_LABEL`
                )
              }}
            />
          }
          checkboxErrorMessage={i18n.t(
            'CARD_SUBSCRIPTION_CHECKBOX_ERROR_MESSAGE'
          )}
          checkboxIcons={{
            checked: IconCheckboxChecked,
            unchecked: IconCheckboxUnchecked
          }}
        />
      )}
    </div>
  )
}

// ✅ separate in methods and avoid doing too many things inside the component
const CHECKBOX_ICONS = { // outside the render, they're not changing never
  checked: IconCheckboxChecked,
  unchecked: IconCheckboxUnchecked
}

// render methods could be even separate and reusable React components
renderCardSubscription () {
  const { i18n } = this.props
  const checkboxErrorMessage = i18n.t('CARD_SUBSCRIPTION_CHECKBOX_ERROR_MESSAGE')

  return (
    <SuiCardSubscription
      checkboxErrorMessage={checkboxErrorMessage}
      checkboxIcons={CHECKBOX_ICONS}
      checkboxLabel={this.renderCheckboxLabel()}
    />
  )
}

renderCheckboxLabel () {
  const { appName, i18n } = this.props
  const innerHTML = i18n.t(`CARD_SUBSCRIPTION_${appName}_CHECKBOX_LABEL`)

  return (
    <span
      className="mt-CardSubscription-checkboxLabel"
      dangerouslySetInnerHTML={{ __html: innerHTML }}
    />
  )
}

render () {
  const {errorMessage} = this.props
  // calculate if we have to show the card and don't do it in the render
  const showCardSubscription = this._hasFailed(errorMessage) && this._isRegistered()

  return (
    <div>
      {showCardSubscription && this._renderCardSubscription()}
    </div>
  )
}



Frequently asked questions 🤔

Should I use it ALWAYS?

Nope. The rule is not meant to be strict but a guide to try to follow as much of possible. Sometimes could be possible that one line is more appropriate for doing more things but still following SLR could surface the possibility of extracting a useful function.

// a typical simple example of doing two things in the same line that could make sense
const result = numbers.map(n => n * 2)

// using SLR™
const multiplyBy2 = n => n * 2 // extract the function, could be used again
const result = numbers.map(multiplyBy2)

Anyway, the idea behind the Single Line Responsibility is to do your best effort to not creating code hard to read by concatenating stuff in a single line. It could make sense in a specific context, just keep it in mind.


But I’ve seen you are doing more than one thing per line! Liar!

As I said, let’s not being pedantic about SLR. The idea behind SLR is trying as much of possible of doing one thing per line but, sometimes, could be good enough to try to limit the things to do in one line. In the next case I’m accessing a position of the array featureIds and using it as a param:

this._findFeatureByValue(featureIds[0])

Surely, we could do the next thing:

const featureValue = featureIds[0]
this._findFeatureByValue(featureValue)

But, what happens when we want to do that inside a conditional in order to check if featureIds have at least one position in order to know if we should extract it? Well, there’re plenty of solutions (extracting to a method, control undefined values…). The idea is not trying to achieve everything in one line but sometimes is just fine having an exception.


But I’m sure there’s some kind of performance penalty!

Make it work. Make it right. Make it fast. But not THAT fast. If your code is not understandable and maintainable, nobody cares if is a little bit faster. Even though performing a lot of actions in a single line surely could be more performant but on a micro scale. Don’t sacrifice readability for micro-optimizations. Instead, focus on real performance boost with better algorithms and implementations. If your code should be micro-optimized (for size, for memory, for whatever) then, feel free to put aside this rule.

Sometimes when I ask somebody about why they’re using a single line for doing a bunch of actions they generally answer me: it’s faster. But, is it? Is the user going to perceive it? Worth it?


My code is going to be too verbose!

Is it? To be honest, I’ve never have had problems with my code being too verbose but I’ve found problems with the opposite. Obviously, is still your responsibility to make sense of following this rule. Again, this is a rule and, as all the rules, they’re exceptions. Be clever and responsible. ☝️


Dude, this is NOT new. I do it all the time!

I know. Tons of programmers are good boys and try to do their best to make their code readable, maintainable and clean. But I liked the idea of giving a name for it. If you want your coworkers to improve their codes, please, feel free to share this article within your company or your social networks. Let’s do lines of code great again!

Comparte el artículo