# Best Practice: Delete Method

**URL:** <https://www.ruby-forum.com/t/best-practice-delete-method/175016>\
**Category:** Rails\
**Created:** [September 5, 2009, 9:09pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016 "2009-09-05T21:09:09Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 5, 2009, 9:09pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/1 "2009-09-05T21:09:09Z")

</div>

Hey,  
I was just wondering what the best practice for this situation would  
be. I’ve got two models(word, definition) both with destroy methods in  
the controllers that just change the model.status to “deleted”. In the  
words controller I’d like it to call the definition\_controller destroy  
method on definition models. And I’m just blanking on how to do this  
like a regular model method. Would I have to double the destory method  
in the model? Then I wouldn’t be adhering to dry.

// words\_controller.rb  
def destroy  
begin  
ActiveRecord::Base.transaction do  
@word = Word.find(params[:id], :include =\> :definitions)  
@word.status = ‘deleted’  
@word.save

```
    @word.definitions.each do |h|
      h.destroy // Actually destorys the record instead of just

```

setting the status to “deleted”  
end  
end  
rescue ActiveRecord::RecordInvalid =\> invalid  
flash[:notice] = ‘Word was not deleted’  
render :action =\> “new”  
end

```
respond_to do |format|
  format.html { redirect_to(words_url) }
end

```

end

// definitions\_controller.rb  
def destroy  
begin  
ActiveRecord::Base.transaction do  
@definition = Definition.find(params[:id])  
@definition.status = ‘deleted’  
@definition.save  
end  
rescue ActiveRecord::RecordInvalid =\> invalid  
flash[:notice] = ‘Definition was not deleted’  
render :controller =\> :words, :action =\> “new”  
end

```
respond_to do |format|
  format.html { redirect_to(words_url) }
end

```

end

Thanks,  
bp

---

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 5, 2009, 9:27pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/2 "2009-09-05T21:27:18Z")

</div>

On Sep 5, 3:07 pm, brianp [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) wrote:

> def destroy  
> end  
> // definitions\_controller.rb  
> end
> 
> ```
> respond_to do |format|
> format.html { redirect_to(words_url) }
> end
> 
> ```
> 
> end
> 
> Thanks,  
> bp

I’m not sure how creating a method that sets the status attribute to  
“deleted” and saves the record would conflict with DRY principles?  
Also, it seems that the specifics of this what-would-be “mark\_deleted”  
or maybe “set\_status(‘deleted’)” method is something the controller  
_probably_ doesn’t care about; more the controller just wants your  
model to perform a specific unit of logic.

Moreover, it doesn’t make sense (as far as I can see) to call a  
separate controller’s actions outside the realm of the HTTP redirect  
dance. Controllers exist to sit in between your user’s request and the  
view/content that they want rendered back, having the appropriate  
models perform whatever actual logic is required to make this happen.

---

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 5, 2009, 10:09pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/3 "2009-09-05T22:09:27Z")

</div>

Your are very right. I shouldn’t actually have the controller changing  
the status at all. I should maybe have the controller call the models  
change status method when called but that is it. And because the  
method will now be in the model it wont be in the controller and I  
wont be breaking dry like I thought of before.

I should have thought of that earlier once i realized destroy was  
doing a little more work then actually destroying.

Thanks for the input, I knew I was looking at it from the wrong angle  
it just felt wrong.

---

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 6, 2009, 9:29am UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/4 "2009-09-06T09:29:07Z")

</div>

2009/9/5 brianp [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid):

> So after what i end up with is a:
> 
> // word.rb, definition.rb  
> Â def set\_status(status)  
> Â Â self.status = status  
> Â end

Arguably set\_status is not a good name, as it requires the caller to  
know about the status field in the model. Something mark\_deleted  
might be a better name.

Colin

---

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 5, 2009, 10:55pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/5 "2009-09-05T22:55:42Z")

</div>

So after what i end up with is a:

// word.rb, definition.rb  
def set\_status(status)  
self.status = status  
end

// word\_controller.rb  
def destroy  
@word = Word.find(params[:id], :include =\> :definitions)

```
begin
  ActiveRecord::Base.transaction do
    @word.set_status("deleted")
    @word.save

    @word.definitions.each do |h|
      h.set_status("deleted")
      h.save
    end
end
rescue ActiveRecord::RecordInvalid => invalid
    flash[:notice] = 'Word was not deleted'
    render :action => "new"
end

respond_to do |format|
  format.html { redirect_to(words_url) }
end

```

end

Which seems much more appropriate.

---

<div class="post-metadata">

**Author:** ![brianp](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/b/94ad74/32.png) [@brianp](https://www.ruby-forum.com/u/brianp)\
**Post date:** [September 7, 2009, 12:35pm UTC](https://www.ruby-forum.com/t/best-practice-delete-method/175016/6 "2009-09-07T12:35:41Z")

</div>

Moreover, can’t we move out explicit save in the controller to model  
like  
this:

## model.rb

def mark\_deleted  
self.update\_attribute(:status, “deleted”)  
end

Thanks,  
Abhinav

–  
अभिनव  
[http://twitter.com/abhinav](http://twitter.com/abhinav)