0

I have the following function in one of my classes

def intraday_time_series(opts)

  ### Guard block ###

  valid_resource = opts[:resource] && [:calories, :steps, :distance, :floors, :elevation].include?(opts[:resource])
  valid_date = opts[:date]
  valid_detail_level = opts[:detailLevel] && %w(1min 15min).include?(opts[:detailLevel])

  raise FitgemOauth2::InvalidArgumentError,
        'Must specify resource to fetch intraday time series data for.'\
        ' One of (:calories, :steps, :distance, :floors, or :elevation) is required.' unless valid_resource

  raise FitgemOauth2::InvalidArgumentError, 'Must specify the date to fetch intraday time series data for.' unless valid_date

  raise FitgemOauth2::InvalidArgumentError,
        'Must specify the data resolution to fetch intraday time series data for.'\
        ' One of (\"1d\" or \"15min\") is required.' unless valid_detail_level

  ### actual logic ###
  resource = opts.delete(:resource)
  date = format_date(opts.delete(:date))
  detail_level = opts.delete(:detailLevel)
  time_window_specified = opts[:startTime] || opts[:endTime]
  resource_path = "user/#{@user_id}/activities/"

  if time_window_specified
    start_time = format_time(opts.delete(:startTime))
    end_time = format_time(opts.delete(:endTime))
    resource_path += "#{resource}/date/#{date}/1d/#{detail_level}/time/#{start_time}/#{end_time}.json"
  else
    resource_path += "#{resource}/date/#{date}/1d/#{detail_level}.json"
  end
  get_call(resource_path)
end

The function has a guard block that checks for any issues with the arguments (the guard block) and if no error is found it does stuff with those arguments.

Now, when I use rubocop to analyze my code, it reports high cyclometic complexity due to the guard blocks. One way I can reduce the complexity is to define another function guard_for_intraday_time_series and move all the guard blocks there. I do not think it as an appropriate solution though because it will populate my code with a guard block for every function that I have in my project.

What is an appropriate way to reduce this complexity, or is it just unavoidable?

Ankit
  • 6,772
  • 11
  • 48
  • 84

1 Answers1

2

You have many intermediate local variables that make the code complicated.

I would refactor it like this:

def intraday_time_series(resource: nil, date: nil,
    detailLevel: nil, startTime: nil, endTime: nil)
  unless %i[calories steps distance floors elevation].include?(resource)
    raise FitgemOauth2::InvalidArgumentError,
      "Must specify resource to fetch intraday time series data for."\
      " One of (:calories, :steps, :distance, :floors, or :elevation) is required."
  end
  unless date
    raise FitgemOauth2::InvalidArgumentError,
      "Must specify the date to fetch intraday time series data for."
  end
  unless %w(1min 15min).include?(detailLevel)
    raise FitgemOauth2::InvalidArgumentError,
      "Must specify the data resolution to fetch intraday time series data for."\
      " One of (\"1d\" or \"15min\") is required."
  end
  resource_path = [
    "user", @user_id,
    "activities", resource,
    "date", format_date(date),
    "1d", detailLevel
  ].join("/")
  if startTime || endTime
    resource_path =
    [resource_path, "time", format_time(startTime), format_time(endTime)].join("/")
  end
  get_call("#{resource_path}.json")
end
sawa
  • 165,429
  • 45
  • 277
  • 381