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