«

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

eggie5 wrote:
> 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


(params[:categories]||[]).each do |id|
@subscriber.subscriptions.create :category_id => id,
:timestamp_start => Time.now.to_s, :is_subscribed => true
end

best,
Dan
--
Posted via http://www.ruby-forum.com/.

Hi,

Am Dienstag, 28. Aug 2007, 23:00:08 +0900 schrieb 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

Maybe:

pc = params[:categories]
pc and pc.each do |id|
...
end

Bertram


--
Bertram Scharpf
Stuttgart, Deutschland/Germany
http://www.bertram-scharpf.de

eggie5 wrote:
> 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
>

params[:categories].to_a.each do ...


lopex

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...

On 28.08.2007 16:09, Bertram Scharpf wrote:
> Hi,
>
> Am Dienstag, 28. Aug 2007, 23:00:08 +0900 schrieb 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
>
> Maybe:
>
> pc = params[:categories]
> pc and pc.each do |id|
> ...
> end

+1

robert