How would I refactor these four `unless` statements?

Let's say I have the following four `unless` statements in a row. At
first I thought about using `if` and `elsif` statements and negate the
condition but I'm not sure which way is suitable/readable/good-practice.

  unless @params.include?(:js_code) || @params.include?(:code_url)
    raise BadParameters, 'You must include at least one...'
  end
  unless @params.include?(:compilation_level)
    raise BadParameters, 'The :compilation_level...'
  end
  unless @params.include?(:output_format)
    raise BadParameters, 'The :output_format...'
  end
  unless @params.include?(:output_info)
    raise BadParameters, 'The :output_info...'
  end

Thanks in advance!

···

--
Posted via http://www.ruby-forum.com/.

For the conditions that are really similar, you could do something like:

   missing_keys = [:compilation_level, :output_format, :output_info] - @params.keys

   unless missing_keys.empty?
     raise BadParameters, "Missing keys: #{missing_keys.inspect}"
   end

-Justin

···

On 06/11/2013 12:38 PM, Rafal C. wrote:

Let's say I have the following four `unless` statements in a row. At
first I thought about using `if` and `elsif` statements and negate the
condition but I'm not sure which way is suitable/readable/good-practice.

   unless @params.include?(:js_code) || @params.include?(:code_url)
     raise BadParameters, 'You must include at least one...'
   end
   unless @params.include?(:compilation_level)
     raise BadParameters, 'The :compilation_level...'
   end
   unless @params.include?(:output_format)
     raise BadParameters, 'The :output_format...'
   end
   unless @params.include?(:output_info)
     raise BadParameters, 'The :output_info...'
   end

Thanks in advance!

If you're using Ruby 2.0 then you have the option of methods with
keyword arguments, which would handle that kind of thing for you.

···

--
Posted via http://www.ruby-forum.com/.

Rafal C. wrote in post #1112088:

Let's say I have the following four `unless` statements in a row. At
first I thought about using `if` and `elsif` statements and negate the
condition but I'm not sure which way is suitable/readable/good-practice.

  unless @params.include?(:js_code) || @params.include?(:code_url)
    raise BadParameters, 'You must include at least one...'
  end
  unless @params.include?(:compilation_level)
    raise BadParameters, 'The :compilation_level...'
  end
  unless @params.include?(:output_format)
    raise BadParameters, 'The :output_format...'
  end
  unless @params.include?(:output_info)
    raise BadParameters, 'The :output_info...'
  end

Thanks in advance!

I think they're fine. When it comes to logical statements for error
handling, keeping it simple, systematic, and easy to understand is a
good thing. Building some larger, more complex expressions just to make
the code shorter will only obfuscate it and require a reader to do more
interpretation in the head to make sense of it.
Possibly, you can define a helper method for the simple cases like this:

def required(name, message)
  unless @params.include?(name)
    raise BadParameters, message
  end
end

and turn three of them into:

required(:compilation_level, 'The :compilation_level...')
required(:output_format, 'The :output_format...')
required(:output_info, 'The :output_info...')

If the messages are completely uniform (except for the parameter name of
course), you can also do it this way:

def required(names)
  names.each do |name|
    unless @params.include?(name)
      raise BadParameters, "The #{name} ..."
    end
  end
end

required([:compilation_level, :output_format, :output_info])

···

--
Posted via http://www.ruby-forum.com/.

As one can see there are many options. Here's another one

MANDATORY = [:js_code, :code_url, ...]
...

missing = MANDATORY.reject do |key|
  @params.include? key
end

raise BadParameters, "These parameters are missing: #{missing.join ", "}"
unless missing.empty?

Cheers

robert

···

On Tue, Jun 11, 2013 at 9:38 PM, Rafal C. <lists@ruby-forum.com> wrote:

Let's say I have the following four `unless` statements in a row. At
first I thought about using `if` and `elsif` statements and negate the
condition but I'm not sure which way is suitable/readable/good-practice.

  unless @params.include?(:js_code) || @params.include?(:code_url)
    raise BadParameters, 'You must include at least one...'
  end
  unless @params.include?(:compilation_level)
    raise BadParameters, 'The :compilation_level...'
  end
  unless @params.include?(:output_format)
    raise BadParameters, 'The :output_format...'
  end
  unless @params.include?(:output_info)
    raise BadParameters, 'The :output_info...'
  end

--
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

Justin Collins wrote in post #1112097:

   missing_keys = [:compilation_level, :output_format, :output_info] -
@params.keys

   unless missing_keys.empty?
     raise BadParameters, "Missing keys: #{missing_keys.inspect}"
   end

Thanks Justin.

···

--
Posted via http://www.ruby-forum.com/.