Can this be more succinct?

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

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

        params[:categories].each do |id|
            @subscriber.subscriptions.create :category_id =>

id, :timestamp_start => Time.now.to_s, :is_subscribed => true
end
end

Yes, you can:

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

Though I personally find the longer form more readable. I read code top
to
bottom, so having the condition for a block at the top of it helps.

HTH,

Felix

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

On 28.08.2007 16:09, Bertram S. wrote:

        end
    end

Maybe:

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

end

+1

robert

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 :params => :category_ids, :method => :post,
:only => :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 :params => :category_ids, :method => :post,
:only => :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…

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs