How can I improve my "reviewname" method

[Jay Lemar]

Hey guys, I am a student and I got the memo (without explanation) that
my following code can be much better. How would you improve my
controller/reviewname method? Thank you for your time!

class ReviewController < ApplicationController

def index
@reviews = Review.all.order(created_at: :desc)
end

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

--
Posted via http://www.ruby-forum.com/.

[Elizabeth McGurty]

1. You don't test to see if Review.find found anything...
2.  You could use begin... rescue ... end to discover the specific error on update_attribute, I think it should be plural
3. Are you using model.errors.any?  , model.errors.empty?
4. Not familiar with your use of sanitize.  I think I would evaluate params with and without sanitize to see how the call is affecting your params
5. How are you managing 200 and 500?  I think that you should return to reviewname in event of a data error to inform the user.

[Sophie Déziel]

Hi,

the reviewname method should be called update.

I think it is prefered to use @review.update_attributes(params.require(:review).permit(:title))

you should put the status code outside of the json response to have them in the head instead of the body. You should render the updated object. You can also use symbols. format.json { render json: @review, status: :ok } 

when the review cannot be updated, you should not use 500 (internal server error) but 422 (unprocessable entity) and give some infos about the error (your model validators does the job for you) for example,  format.json { render json: @review.errors, status: :unprocessable_entity }