# Calling controller actions in a for loop

**URL:** <https://www.ruby-forum.com/t/calling-controller-actions-in-a-for-loop/111684>\
**Category:** Rails\
**Created:** [August 9, 2007, 8:28pm UTC](https://www.ruby-forum.com/t/calling-controller-actions-in-a-for-loop/111684 "2007-08-09T20:28:31Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![cmaxvv](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/85f322/32.png) [@cmaxvv](https://www.ruby-forum.com/u/cmaxvv)\
**Post date:** [August 9, 2007, 8:28pm UTC](https://www.ruby-forum.com/t/calling-controller-actions-in-a-for-loop/111684/1 "2007-08-09T20:28:31Z")

</div>

I have Story objects that each “has\_many” Pictures (and Stories  
belongs\_to Article but that’s not relevant to this question). The  
picture controller has a ‘delete’ method that clears out the saved  
picture from the public folder and then deletes the record from the db  
with @picture.destroy.

In the Story controller’s delete method, i want to call  
PictureController’s delete method on every picture owned by the story  
before calling destroy on the story itself. Is this possible? Or does  
the delete method need to belong to the Picture model class (as opposed  
to the controller)?

Currently i’m doing this:

#in StoryController

def delete  
@story = Story.find(params[:id])  
@article = @story.article  
for picture in @story.pictures  
PictureController.delete picture.id  
end  
@story.destroy  
redirect\_to :controller =\> “article”, :action =\> “show”, :id =\>  
@article  
end

But it’s complaining because delete isn’t a class method of  
PictureController (quite correctly). Does anyone know what i should do  
here? Thanks!  
max

---

<div class="post-metadata">

**Author:** ![cmaxvv](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/85f322/32.png) [@cmaxvv](https://www.ruby-forum.com/u/cmaxvv)\
**Post date:** [August 9, 2007, 10:23pm UTC](https://www.ruby-forum.com/t/calling-controller-actions-in-a-for-loop/111684/2 "2007-08-09T22:23:57Z")

</div>

I’m not sure of the best way to describe it, but this is a violation of  
the  
MVC (Model View Controller) paradigm that Rails is built around.  
Controllers  
are supposed to do nothing but tell the Model what to do, whether to  
give up  
information that gets sent to the view or to perform actions on it’s  
internal data. In this case, your controller should only care about  
Story  
objects (as it is the StoryController). It should not care, or even know  
about Pictures. Only the PictureController should care about Pictures,  
and  
even then that is irrelevant to your current situation.

This method is meant to delete the selected story, so do only and  
exactly  
that:

def delete  
Story.find(params[:id]).destroy  
redirect\_to …  
end

Then, it’s up to the Story object to take care of cleaning out what it’s  
responsible for:

class Story \< AR::Base  
has\_many :pictures, :dependent =\> :destroy  
end

But it’s up to the Picture to care about what’s on the file system:

class Picture \< AR::Base  
belongs\_to :story

def destroy  
[do file system clean up]  
end  
end

This is a “best-practice” that’s becoming more and more prevalent known  
as  
Skinny Controllers or Fat Models. The Controller methods should be  
extremely  
short, only dealing with telling models what to do and manipulating  
control  
flow through the application. This allows multiple controllers to access  
similar functionality without violating the oft-mentioned ideology of  
DRY  
(Don’t Repeat Yourself), and also makes the whole code base easy to test  
(these ideas actually have come from people practicing and learning Test  
Driven Development).

So hopefully I’ve explained the What and the Why. If you have any  
questions,  
please don’t hesitate to ask.

Jason

---

<div class="post-metadata">

**Author:** ![cmaxvv](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/85f322/32.png) [@cmaxvv](https://www.ruby-forum.com/u/cmaxvv)\
**Post date:** [August 10, 2007, 1:39am UTC](https://www.ruby-forum.com/t/calling-controller-actions-in-a-for-loop/111684/3 "2007-08-10T01:39:01Z")

</div>

That’s a great explanation, thanks Jason. I was asking a friend about  
this tonight and funnily enough he recommended exactly the same thing:  
in the story model i say

has\_many :pictures, :dependent =\> :destroy

and then in the picture model i say

before\_destroy :delete\_from\_disc

where “delete\_from\_disc” is my picture model method to clear out the  
files associated with the deleted record.

Thanks for the help!  
max