Validating the presence of properties in a document

Question

How can I reduce cyclomatic complexity of validation and refactor it better way?

Class Document
  def initialize(opts)
    @opts = opts
    return ArgumentError unless valid?
  end

  def valid?
    return false if @opts.nil?
    return false if @opts[:property_1].nil?
    return false if @opts[:property_2].nil?
    return false if !@opts[:property_4].nil? && @opts[:property_3].nil?
    # etc. 12 times more, otherwise valid will return true
    true
  end

  def usage_example
   if @opts[:property_3]
      "#{@opts[:property_3]} #{@opts[:property_1]}"
   else
      "#{@opts[:property_4]} #{@opts[:property_2]}"
   end
  end
end

I am trying to avoid duplication of return false if and somehow perhaps list parameters that I want to verify

Possible interpretation of what I trying to reach is "verify set of rules before processing file"

Answer 1

If your tests are about mandatory members in @opts you can do this:

MandatoryProperties = [:property_1, :property_2, ...]

def valid?
  @opts && MandatoryProperties.all? { |k| !@opts[k].nil? }
end

Answer 2

There are a few things you can do here to improve this code, but with any sort of refactoring or any attempt of reducing complexity, we have to evaluate why. It may very well be the case that this method isn't at fault for its own complexity, but is this complex due to a failure elsewhere in the system of keeping their data correct.

That aside, there are still a few things you could do to clean up this code a little

Explicitly define all the required parameters and use that to iterate over.

REQUIRED_PROPERTIES = %i(prop_1 prop_2 prop_3 prop_4)
REQUIRED_PROPERTIES.any? { |x| @opts[x].nil? } #=> true if any are nil or false if all are not nil

Use hash#fetch instead of [] so you can supply a default value for possible missing keys

return false if @opts[:property_1].nil?
return @opts.fetch(:property_1, false) #only return false if :property_1 is not defined
#fetch only returns false if :property_1 doesn't exist, not if it has been set to nil

Remember that in ruby nil and false are both false-y

def valid?
  @opts && @opts[:property_1] && @opts[:property_2]
  #will return true only if @opts and properties are non nil values
end

Answer 3

You could write that as

def valid?
  @opts && !@opts[:property].nil? ? true : false
end

but usually it would be good enough to return a truthy or falsy value instead of a Boolean:

def valid?
  @opts && !@opts[:property].nil?
end