Refactor controller action code DRY

0
votes

I am using indeed_api to retrieve jobs from Indeed API, but because Indeed only allows 25 results per query, i have come up with this code in my controller to get all jobs and list them all on one page:

    @jobs = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", limit: "25")
    @results = @jobs.results

    if @jobs.total_results > 25
        @jobs2 = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", start: "25", limit: "25")
        @results += @jobs2.results
    end

    if @jobs.total_results > 50
        @jobs3 = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", start: "50", limit: "25")
        @results += @jobs3.results
    end

    if @jobs.total_results > 75
        @jobs4 = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", start: "75", limit: "25")
        @results += @jobs4.results
#and so on...

This is rather ugly and definitely not a rails way to do it. I mean, the controller could spread out to hundreds of lines. Is there a way to put this code in a loop or refactor it in any other way?

3
ruby-on-railsrubyrefactoring

3 Answers

0
votes

You can do something like:

@jobs = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", limit: "25")
@results = @jobs.results
total_results = @jobs.total_results

if total_results.size > 25
  25.step(total_results, 25) do |step|
    r = IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", limit: "25", start: step)
    @results = @results.concat(r)
  end
end

It's a bit dirty of a solution, but I think that you could refactor it a bit. Basically, the magic is in the step method which invokes block with the sequence of numbers starting at total_results, incremented by 25 on each call. You can read more about Numeric#step here.

Hope that helps!

0
votes

I think you should move this out of controller, it's a good practice to move this to some service using background jobs like sidekiq etc.

If you want a loop:

batches = @jobs.total_results / 25
@results = []
(0..batches).do |batch|
  @results << IndeedAPI.search_jobs(co: "au", l: "sydney", radius: "100", sort: "date", start: (batch * 25).to_s, limit: "25")
end
0
votes

I would do something like this:

query = { co: 'au', l: 'sydney', radius: 100 }
total = IndeedAPI.search_jobs(query).total_results

per_page = 25
pages = (total.to_f / per_page).ceil

@results = pages.times.flat_map do |page|
  IndeedAPI.search_jobs(
    query.merge(sort: 'date', start: per_page * page, limit: per_page
  ).results
end