Richlewis
Richlewis

Reputation: 15374

How to write a guard clause with multiple conditions in Ruby?

After running Rubocop against this code I am getting

Use a guard clause instead of wrapping the code inside a conditional expression.

So from what I have read a "Guard Clause" will bail out of the method if the condition is not met, so we don't have to waste time going through extra conditionals, please correct me if my understanding is not correct.

My question is though how would i use a guard statement with multiple conditions

def auth_creds
  if %w(test1 qa demo ci).include? ENV['ENV']
    { key: 'key1', secret: 'secret1' }
  elsif ENV['ENV'] == 'live'
    { key: 'key2', secret: 'secret2' }
  else
    fail 'Unable to set key/secret'
  end
end

Thanks

Upvotes: 4

Views: 4422

Answers (3)

Stefan
Stefan

Reputation: 114158

It all depends on your actual code, but with the given snippet, you could use a guard clause to ensure a valid ENV['ENV'] value:

VALID_ENVS = %w(test1 qa demo ci live)

def auth_creds
  fail 'invalid environment' unless VALID_ENVS.include? ENV['ENV']

  if ENV['ENV'] == 'live'
    { key: 'key2', secret: 'secret2' }
  else
    { key: 'key1', secret: 'secret1' }
  end
end

As noted by Sergio Tulentsev, storing your credentials in ENV (instead of the env's name) would probably be better:

def auth_creds
  { key: ENV.fetch('KEY'), secret: ENV.fetch('SECRET') }
end

fetch will raise a KeyError if the given key is not found in ENV.

Upvotes: 3

mhutter
mhutter

Reputation: 2916

Guard clauses generally go like this:

def do_something
  return 'x' if some_condition?
  # other code
end

So your code could be rewritten as

def auth_creds
  return { key: 'key1', secret: 'secret1' } if %w(test1 qa demo ci).include? ENV['ENV']
  return { key: 'key2', secret: 'secret2' } if ENV['ENV'] == 'live'

  fail 'Unable to set key/secret'
end

However, this is quite ugly, and now rubocop will complain about lines that are too long. So let us reword the code to describe its intention:

def auth_creds
  return { key: 'key1', secret: 'secret1' } if test_env?
  return { key: 'key2', secret: 'secret2' } if live_env?

  fail 'Unable to set key/secret'
end

private # omit if `auth_creds` is also private

def test_env?
  %w(test1 qa demo ci).include? ENV['ENV']
end

def live_env?
  ENV['ENV'] == 'live'
end

Bonus Points: Extract %w(test1 qa demo ci) into a constant!

Double Bonus Points: (thanks @Sergio Tulentsev) Get your environment-specific (and probably sensitive!!!) credentials out of your code! If using Rails, put it in a secrets.yml, else use one of the many great gems for this:


A word about Rubocop: Take its advice with a grain of Salt. For example, your code is not really a case for a guard clause, it's just returning data based on conditions. So instead you could also try refactoring the code as a case expression.

And sometime, Rubocop just talks trash :-) (not on purpose, but "measuring" code style is hard!)

Upvotes: 2

Sergio Tulentsev
Sergio Tulentsev

Reputation: 230326

Your snippet is not a valid example for "guard clauses". There's nothing to guard against. It's just selecting data. It'd look better as case/when but if chain is fine too.

def auth_creds
  case ENV['ENV']
  when 'test1', 'qa', 'demo', 'ci'
    { key: 'key1', secret: 'secret1' }
  when 'live'
    { key: 'key2', secret: 'secret2' }
  else
    fail 'Unable to set key/secret'
  end
end

Guard clause (or as I call them, early returns) are used when the whole body of the method is wrapped in a conditional.

def process_project
  if project
    # do stuff
  end
end

The method won't do anything unless there's a project. So it makes for more readable code if we reduce the nesting here.

def process_project
  return unless project

  # do stuff with project
end

Again, not every if in your code can/should be transformed to this form. Only where it's appropriate.

Upvotes: 8

Related Questions