can this be more succinct?

Discussion in 'Ruby' started by eggie5, Aug 28, 2007.

  1. eggie5

    eggie5 Guest

    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, Aug 28, 2007
    #1
    1. Advertising

  2. 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/.
    Daniel Lucraft, Aug 28, 2007
    #2
    1. Advertising

  3. 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
    Bertram Scharpf, Aug 28, 2007
    #3
  4. 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
    Marcin Mielżyński, Aug 28, 2007
    #4
  5. 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,
    :eek: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 :params => :category_ids, :method => :post,
    :eek: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...
    Florian Aßmann, Aug 28, 2007
    #5
  6. 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
    Robert Klemme, Aug 28, 2007
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Michael
    Replies:
    4
    Views:
    396
    Matt Hammond
    Jun 26, 2006
  2. Robert Klemme

    With a Ruby Yell: more, more more!

    Robert Klemme, Sep 28, 2005, in forum: Ruby
    Replies:
    5
    Views:
    202
    Jeff Wood
    Sep 29, 2005
  3. eggie5
    Replies:
    9
    Views:
    102
  4. RichardOnRails

    Can this code be more succinct

    RichardOnRails, Apr 29, 2009, in forum: Ruby
    Replies:
    9
    Views:
    104
    7stud --
    Apr 30, 2009
  5. Bob Rashkin
    Replies:
    5
    Views:
    62
    Dennis Lee Bieber
    Dec 23, 2013
Loading...

Share This Page