How would you improve the following controller method?

I made this code but someone told me it was flawed
How would you improve this piece of code in the reviewcontroller (mainly
the namereview method)?

class ReviewController < ApplicationController
before_action :set_reviews, only: [:show, :edit, :update, :destroy]

def index
@reviews = Review.all
end

def namereview
@review = Review.find(params[:id])
if @review.update_attribute(:description,
sanitize(params[:description
]))
format.json { render json: { status: 200 } }
else
format.json { render json: { status: 500 } }
end
end
end

Thanks in advance

On Wednesday, June 3, 2015 at 6:31:41 AM UTC+1, fizzi wrote:

I made this code but someone told me it was flawed
How would you improve this piece of code in the reviewcontroller (mainly
the namereview method)?

Looking just at that method:

  • I wouldn’t call it namereview - that seems to suggest that it sets a
    review name but it doesn’t. I’d call it update, and make it behave like
    a
    normal update method (i.e. it should use params[:review][:description]).
    You can leave it only updating the description attribute, but at least
    bring the semantics closer to the ‘normal’ update action

  • 422 is the usual http status for failed validations

  • your code seems to let anyone edit any review - not sure if that is
    appropriate.

Fred

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs