-1
\$\begingroup\$

This sample code is correct and no error however this only good if you only have few hundreds or thousand datas but if you have million queries this takes time how we can improve this algorithm

  namespace :auto_status do
    desc "Add Status to the Tracking code Automaticaly"
    task make_status: :environment do
    @product_trackers = ProductTracker.all
    @statuses = Status.all
    @date = DateTime.now.utc
    @product_trackers.each do |product_tracker|

            if (Status.where(product_tracker_id: product_tracker.id).count == 1 && (((Time.parse(DateTime.now.to_s) - Time.parse(Status.where(product_tracker_id: product_tracker.id).first.created_at.to_s))/1.days).round) == 1)
                @links = Status.new
                @links.order_status = 1
                @links.product_tracker_id = product_tracker.id

                @links.save
            elsif (Status.where(product_tracker_id: product_tracker.id).count == 2 && (((Time.parse(DateTime.now.to_s) - Time.parse(Status.where(product_tracker_id: product_tracker.id).first.created_at.to_s))/1.days).round) == 2)

                @links = Status.new
                @links.order_status = 2
                @links.product_tracker_id = product_tracker.id

                @links.save
            elsif (Status.where(product_tracker_id: product_tracker.id).count == 3 && (((Time.parse(DateTime.now.to_s) - Time.parse(Status.where(product_tracker_id: product_tracker.id).first.created_at.to_s))/1.days).round) == 5)

                @links = Status.new
                @links.order_status = 3
                @links.product_tracker_id = product_tracker.id

                @links.save
            elsif (Status.where(product_tracker_id: product_tracker.id).count == 4 && (((Time.parse(DateTime.now.to_s) - Time.parse(Status.where(product_tracker_id: product_tracker.id).first.created_at.to_s))/1.days).round) == 6)
                @links = Status.new
                @links.order_status = 4
                @links.product_tracker_id = product_tracker.id

                @links.save
            end
    end

  end

end
\$\endgroup\$
  • 2
    \$\begingroup\$ Your title and question text should describe what the code actually does, not just what you want from a review. \$\endgroup\$ – forsvarir Dec 7 '16 at 9:13
  • \$\begingroup\$ Writing Fast Ruby \$\endgroup\$ – radarbob Dec 7 '16 at 15:20
4
\$\begingroup\$

You have a lot of Code repeated one of the important rules is to be DRY(Don't repeat yourself), check the following, what i did is only DRY the code:

namespace :auto_status do
  desc "Add Status to the Tracking code Automaticaly"
    task make_status: :environment do

    ProductTracker.select(:id).find_each do |product_tracker|
      status = Status.where(product_tracker_id: product_tracker.id)
      count = status.count
      next if count == 0
      days_diff = ((Time.parse(DateTime.now.to_s) - Time.parse(status.first.created_at.to_s))/1.days).round

      if ( (count == 1 && days_diff == 1) || 
           (count == 2 && days_diff == 2) || 
           (count == 3 && days_diff == 5) ||
           (count == 4 && days_diff == 6) ){
            Status.create(product_tracker_id: product_tracker.id, order_status: count)
           }
    end
  end
end  

Notice here

  • Used find_each as if you have many ProductTracker that will blow the memory, find_each will load them in batches.
  • Select only id from ProductTrackerthat is the only thing needed here.
  • Skip the loop if no status so save time from do the next conditions while no need.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.