Calling controller actions in a for loop

Rails
1

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

2

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

3

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