Hi eggie5,
How can this be more succinct? Can I somehow put the nil check inline
with the each?
unless(params[:categories].nil?)
params[:categories].each do |id|
@subscriber.subscriptions.create :category_id =>
id, :timestamp_start => Time.now.to_s, :is_subscribed => true
end
end
Just a minor thingy, rename timestamp_start column to created_at and
Rails does its magic. So you can write (categories replaced with
category_ids!):
(params[:category_ids] || []).each do |category_id|
@subscriber.subscriptions.create
category_id => category_id,
:is_subscribed => true)
end
You can also put a verify in the class definition of your controller:
class SubscriptionsController < ApplicationController
verify
arams => :category_ids, :method =>
ost,
nly => :subscribe,
:redirect_to => :back
def subscribe
# ...
# both String and Array do have the each method...
params[:category_ids].each do |category_id|
@subscriber.subscriptions.create
category_id => category_id,
:is_subscribed => true)
end
# ...
end
# ...
end
You can even move this whole bunch of code to the subscribers model itself:
class Subscriber < ActiveRecord::Base
# This methods accepts multiple category ids in form of:
# instance.subscribe 1, 2, 4, ...
def subscribe(*category_ids)
# wrap insertion of multiple records in a transactions is generally
# a good idea...
ActiveRecord::Base.transaction do
category_ids.each do |category_id|
subscriptions.create
category_id => category_id,
:is_subscribed => true)
end
end
end
end
class SubscriptionsController < ApplicationController
verify
arams => :category_ids, :method =>
ost,
nly => :subscribe,
:redirect_to => :back
def subscribe
# ...
@subscriber.subscribe(*params[:category_ids])
# ...
end
# ...
end
Or extend the association with a module, see AssociationExtensions in
ActiveRecord::Associations::ClassMethods. in this case you could write
something like:
instance.subscriptions.category_ids = category_ids
This encapsulate all methods in a module that are only important for the
association itself - it's clean!
If you don't use ActionPack and ActiveRecord at all don't mind this post...
Regards
Florian
P.S.
Beware, this code was not run through any functional or unit tests...
