Handling multiple carts in rails -

i'm wondering if there shorter solution problem.

i'm building multiple shop system, means website different shops have 1 open cart each shop.

this cart can owned guest or user.

the cart/order should created when item added.

the following definition placed in application controller.

 def find_order_by_shop(shop)    shop = shop.find(shop.to_i) if !(shop.class == shop)    if session["order_id_for_shop_#{shop.try(:id)}"]      order = order.find_by_id_and_state(session["order_id_for_shop_#{shop.id}"],'open')      if order        if current_user          current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => session["order_id_for_shop_#{shop.id}"]).delete_all          order.update_attribute(:user_id, current_user.id)          order = current_user.orders.where(:shop_id => shop.id , :state => 'open').first          if order            # delete exept first            current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => order.id).delete_all          else            # create new            order = current_user.orders.new(:shop_id => shop.id , :state => 'open')          end        end      else        order = order.new(:shop_id => shop.id, :state => 'open')      end    else      if current_user         order = current_user.orders.where(:shop_id => shop.id , :state => 'open').first         if order           current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => order.id).delete_all         else           order = current_user.orders.new(:shop_id => shop.id , :state => 'open')         end       else         # guest         order = order.new(:shop_id => shop.id , :state => 'open')       end    end     session["order_id_for_shop_#{shop.try(:id)}"] = order.id    return order  end 

updating session while adding item cart/order

 ...    def create # add item order      @order = find_order_by_shop(params[:shop_id].to_i)      @order.save # save cart      session["order_id_for_shop_#{params[:shop_id]}"] = @order.id  ... 

this doesn't seem correct rails way.

any suggestions ?

i don't know 'shorter', glancing @ code make few recommendations.

each change make, test code. have unit , functional tests, if not check expected behaviour in browser.

try split logic sensible, small, tight units sensible names. turn them methods. so, may find problems or points optimise. example:

if order   if current_user     current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => session["order_id_for_shop_#{shop.id}"]).delete_all     order.update_attribute(:user_id, current_user.id)     order = current_user.orders.where(:shop_id => shop.id , :state => 'open').first     if order       # delete exept first       current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => order.id).delete_all     else       # create new       order = current_user.orders.new(:shop_id => shop.id , :state => 'open')     end   end else   order = order.new(:shop_id => shop.id, :state => 'open') end 

as there's 'if order' conditional around whole block, internal 'if order' redundant , whole 'else' branch can removed:

if order   if current_user     current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => session["order_id_for_shop_#{shop.id}"]).delete_all     order.update_attribute(:user_id, current_user.id)     order = current_user.orders.where(:shop_id => shop.id , :state => 'open').first     # delete except first     current_user.orders.where(:shop_id => shop.id , :state => 'open').where.not(:id => order.id).delete_all   end else   order = order.new(:shop_id => shop.id, :state => 'open') end 

now can split functionality small, reusable blocks:

def order_from_shop(shop_id)   order.new(:shop_id => shop_id, :state => 'open') end 

look through code , can see @ least 2 places can use method.

notice there no return statement. ruby/rails 'way' allow automatic return kick in - result of last statement in method returned without explicitly declaring it. can apply end of main method:

  ...   session["order_id_for_shop_#{shop.try(:id)}"] = order.id   order end 

back rest of code, start extracting more methods, like:

def user_order_from_shop(shop_id)   current_user.orders.where(:shop_id => shop.id , :state => 'open').first end 

there's few places can use that, too.

encapsulate if statements in small methods, of form:

def method   if xxx     a_method   else     a_different_method   end end 

according sandi metz, no method should have more 5 lines. don't have extreme it's useful so.

eventually you'll have reads lot more english, easier read , determine behaviour of @ glace. @ point may notice lot more duplication or unnecessary, dead chunks of unreachable code. ruthless both.

finally, whole thing looks needs own class.

# app/services/shop_order_query.rb class shoporderquery   attr_accessor :shop, :order, :current_user    def initialize(shop, order, current_user)     self.shop = shop     self.order = order     self.current_user = current_user   end    def find_order_by_shop     ...     ...   end    private    # support methods main look-up    def order_from_shop(shop_id)     ...   end   ...   ... end 

then call with

shoporderquery.new(shop, order, current_user).find_order_by_shop 

then it's nicely tucked away, , usable wherever can pass shop, order , current user... , it's not cluttering controller.

for further reading, blog posts on making thin controllers, extracting service objects , sandi metz's rules. also, buy sandi's book on ruby oo programming.