Ruby on Rails - Compare Active Record Result to Array Containing New Instansiated Objects

Question

In my Rails app I am doing two things - comparing an Active Record result to an array built from a JSON response from external API.

First step is checking which results are in the database and not in the API and I do this as follows:

def self.orphaned_in_db
  db_numbers = self.find(:all).map{|x| x.number}
  listed_numbers = self.all_telco.map{|x| x.number} # gets JSON from API
  orphaned_numbers = db_numbers - listed_numbers
  orphaned_results = self.find_all_by_number(orphaned_numbers)
  return orphaned_results
end

This is the new version as the old version was taking far too long after the result sets of each increased dramatically in the last few weeks.

 # def self.orphaned_in_db
  # old version
  # db_numbers = self.find(:all)
  # listed_numbers = self.all_telco
  # orphaned_numbers = []
  # db_numbers.each do |db|
    # scan = listed_numbers.select{ |l| l.number == db.number}
    # orphaned_numbers.push(db) if scan.empty?
  # end
  # return orphaned_numbers
# end

I am now finding it tricky to do the opposite of this - find numbers in the API array that are not in my database table.

def self.orphaned_in_telco
  db_numbers = self.find(:all).map{|x| x.number}
  all_numbers = self.all_telco
  listed_numbers = all_numbers.map{|x| x.number}
  orphaned_numbers = listed_numbers - db_numbers
  orphaned_results = # how to filter all_numbers by orphaned_numbers?
  return oprhaned_results
end

Again, the old way that now fails to work because it's so slow:

# def self.orphaned_in_telco
  # original, inefficient way
  # db_numbers = self.find(:all)
  # listed_numbers = self.all_telco
  # orphaned_numbers = []
  # listed_numbers.each do |l|
    # scan = db_numbers.select{ |db| l.number == db.number}
    # orphaned_numbers.push(l) if scan.empty?
  # end
  # return orphaned_numbers
# end

I am finding this difficult because it's the same view and partial that was used before to display these orphaned numbers (both iterations) but it's legacy code so I've never seen it working in action but I'm just confused how it worked before with Active Record results and a normal array.

The view is just:

<%= render :partial => 'list_item', :collection => @telco_numbers ) %>

Where @telco_numbers is set to the return value of the above methods. (@telco_numbers = TelcoNumber.orphaned_in_telco params[:page])

The partial is as follows:

<tr>
<td>

<%= (link_to list_item.organisation.name.truncate(30), :controller => 'organisation', :action => 'update', :id => list_item.organisation.id) if list_item.organisation %>
</td>
<td class="centre"> <%= link_to ((list_item.countrycode == "44" ? Telco.format_uk_number(list_item.number) : "+#{list_item.countrycode} #{list_item.number}")), {:action => 'update', :id => list_item.id} %></td>
<td class="centre"><%= list_item.route_technology %></td>
<td><%= list_item.route_destination if list_item.route_destination %></td>
<td class="centre">
    <% if !list_item.new_record? %>
    [ <%= link_to 'Edit', {:action => 'update', :id => list_item.id} %> ]
    <% else %>
    [ <%= link_to 'Add New', {:action => 'update', :telco_number => list_item.attributes } %> ]

    <% end %>
</td>

So as I understand it, for the the version I am trying to fix, instead of having an Edit action, it would have an Add New link since it's not in my database table yet so I am just trying to figure out how to refactor the inefficient version so that it will still work with the shared view.

If it helps, the format of the JSON API response is :

[{"country_code":"44","number":"1133508889","block":null,"type":"Legacy","SMS":"0"},

So only country_code and number correspond to columns in my database table, the rest isn't necessary, hence the if statements in the partial to only show certain parameters if they are available.

UPDATE

After changing the method to the following as suggested by Chris Vo, it finally works after taking ages to complete but it's still not quite right.

def self.orphaned_in_telco
  # original, inefficient way
  db_numbers = self.find(:all)
  listed_numbers = self.all_telco
  orphaned_numbers = listed_numbers - db_numbers
  orphaned_results = []
  orphaned_numbers.each do |n|
    item = self.new()
    item.id = n
    orphaned_results.push(item)
  end

  return orphaned_results

end

The number column in my view html table just contains the + character and the Add New link doesn't have any value for the countrycode and number url parameters (link url is correct and all parameters are in the query string but they are all empty).

Some of the methods in my model:

def self.max_search_results
  return @@max_search_results
end

#for pagination
def self.per_page
  20
end

def self.some_telco(per_page, page = 1)
  page = 1 if page.nil?
  api_call = TelcoApiv3.new("post", "/numbers/#{TelcoApiv3.account_id}/allocated/all")
  listed_numbers = TelcoApiv3.poll(api_call.response["link"])
  return listed_numbers.collect do |ln| 
    ln.store("countrycode", ln["country_code"])
    TelcoNumber.new ln
  end
end

def self.all_telco(page = 1)
  listed_numbers = some_telco(@@max_nlist_results, page)
  if listed_numbers.length == @@max_nlist_results
    return listed_numbers.concat(all_telco(page + 1))
  else
    return listed_numbers
  end
end

Answer 1

What if, for the orphaned_in_telco method, after you find the orphaned_numbers you create an instance of your model for every number in that set and then push it in a table to return them... or at least something in that direction. e.g.

orphaned_results = []
orphaned_numbers.each do |n|
 item = self.new()
 item.id = n
 orphaned_results.push(item)
end

return orphaned_results

And then from the partial when you call Add new you would only need to call save on that instance.

This way you won't have the problem of Active Record and array for the partial, since you will be returning an array of Active record instances.

Also, my suggestion for speeding up things, would be to use a Hash to store the keys/numbers.

Hope it helps!

UPDATE

In order to have the countrycode and speed up things a little, I will continue with my Hash suggestion:

So, let's start from your initial implementation:

#this returns everything from the API
all_numbers = self.all_telco

#this returns a Hash in the form {:number => :country_code}
listed_numbers = Hash[all_numbers.map{|x| [x.number,x.country_code]}] 
#so now you can do
orphaned_numbers = listed_numbers.keys - db_numbers
orphaned_results = []
orphaned_numbers.each do |n|
  item = self.new()
  item.number = n
  item.countrycode = listed_numbers[n]
  orphaned_results.push(item)
end

return orphaned_results

This should give it a boost and also send the country_code to the front side.