# Can this be more succinct?

**URL:** <https://www.ruby-forum.com/t/can-this-be-more-succinct/113190>\
**Category:** Ruby\
**Created:** [August 28, 2007, 4:01pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190 "2007-08-28T16:01:24Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 4:01pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/1 "2007-08-28T16:01:24Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 4:07pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/2 "2007-08-28T16:07:08Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 4:10pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/3 "2007-08-28T16:10:26Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 4:16pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/4 "2007-08-28T16:16:26Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 4:36pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/5 "2007-08-28T16:36:31Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 7:36pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/6 "2007-08-28T19:36:10Z")

</div>

On 28.08.2007 16:09, Bertram S. wrote:

> > ```
> > end
> > end
> > 
> > ```
> 
> Maybe:
> 
> pc = params[:categories]  
> pc and pc.each do |id|  
> …  
> end

+1

robert

---

<div class="post-metadata">

**Author:** ![eggie5](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/e/7c8e57/32.png) [@eggie5](https://www.ruby-forum.com/u/eggie5)\
**Post date:** [August 28, 2007, 6:44pm UTC](https://www.ruby-forum.com/t/can-this-be-more-succinct/113190/7 "2007-08-28T18:44:54Z")

</div>

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…
