DEV Community

Chakrit Wichian for PRODIGY9

Posted on

Comparing Boolean Expression to True

This has to be the oldest trick in the book in the sense that only really early beginner make these mistakes.

Instead of

class User
  attr_reader :username
  attr_reader :email

  def is_valid?
    valid = !self.username.blank? && !self.password.blank?
    valid == true
  end
end
Enter fullscreen mode Exit fullscreen mode

Do this

class User
  attr_reader :username
  attr_reader :email

  def is_valid?
    !self.username.blank? && !self.password.blank?
  end
end
Enter fullscreen mode Exit fullscreen mode

Why?

Boolean expressions eventually resolve to either true or false. Comparing them again to true wastes computation and adds extra cognitive load without actually adding to the readability or explicitlness.

Why Not?

Sometimes you don't always have a binary true or false values. For example, you can have undefined, null or nil values in other languages. In which case, comparing them to true do have a use because you want to avoid the other two (or three) possibility of the value not existing, or the value being false at the same time.

let result = render(template);                  // default options `{}`
result = render(template, { escape: true });    // explicit options

function render(content, options = {}) {
  if (options['escape'] === true) {
    content = htmlEscape(content)
  }

  // ...
}
Enter fullscreen mode Exit fullscreen mode

It may, however, confuses the reader because the absence of value is implicit in this context. Calling out implicitness usually improves readability but reduce succintness and adds conginitive load. So depending on your situation, shorter may be better or explicit may be better.

One way to make this explicit is to split apart the two concerns in the code:

  1. Checking for absence of value
  2. Checking for the value itself

Like so:

function render(content, options = {}) {
  if (typeof options['escape'] !== 'undefined' && options['escape']) {
    content = htmlEscape(content)
  }

  // ...
}
Enter fullscreen mode Exit fullscreen mode

This code calls out to the reader to be on the lookout for the absence of values in the options hash. That not all options maybe passed into the function.

Alternatively, we can eliminate 1) entirely by supplying defaults:

function render(content, options = {}) {
  if (!options.hasOwnProperty('escape')) {
    options['escape'] = defaults['escape']
  }

  // now we are sure the 'escape` option will always be specified
  // our check is now straightforward
  if (options['escape']) {
    content = htmlEscape(content)
  }

  // ...
}
Enter fullscreen mode Exit fullscreen mode

An experienced coder will also notice that the pattern of "applying defaults" can be made more generic and applied uniformly throughout the codebase. This is exactly the usecase of lodash's _.defaults() method which we can use to make the code more succinct:

function render(content, options = {}) {
  _.defaults(options, defaults); // eliminate any "absence of value" cases

  if (options['escape']) {
    content = htmlEscape(content)
  }

  // ...
}
Enter fullscreen mode Exit fullscreen mode

We can avoid Lodash entirely and use null-coalescing operator ?? if your language supports them:

function render(content, options = {}) {
  if (options['escape'] ?? false) {
    content = htmlEscape(content)
  }

  // ...
}
Enter fullscreen mode Exit fullscreen mode

Top comments (0)