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
