Search code examples
ruby-on-railsrubymethodsupdatesrating

Ruby on Rails - Undefined methods for NilClass


I'm creating a picture-rating app where users can click on pictures and rate them on a scale from 1 to 5. I'm trying to calculate the average rating of a picture. Before when users clicked on a rating value, that value became the picture's rating.

Rating: 5

If a user clicked on 1, the rating would change to 1

Rating: 1

When reality, the rating should have been 3.

(5 + 1) / 2
=> 3

Here's what I've accomplished so far in implementing this feature.

I added a migration to create two new columns for my Pictures Table

rails g migration AddRatingsToPictures ratings_count: integer, rating_total: integer

Both the new attributes, ratings_count and rating_total are integer types, meaning they are assigned a nil value at default.

p = Picture.first
p.attribute_names
=> ['id', 'title', 'category', 'stars', 'updated_at', 'created_at', 
'ratings_count', 'rating_total']
p.ratings_count
=> nil
p.rating_total
=> nil

My only problem is the NilClass Error.

Here is my update method in my PicturesController.

def update
  @picture = Picture.find(params[:id])
  @picture.ratings_count = 0 if @picture.stars.nil?
  @picture.rating_total = @picture.stars
  @picture.rating_total += @picture.stars if @picture.stars_changed?
  @picture.ratings_count += 1 if @picture.rating_total_changed?
  if @picture.update_attributes(picture_params)
    unless current_user.pictures.include?(@picture)
      @picture = Picture.find(params[:id])
      current_user.pictures << @picture
      redirect_to @picture, :flash => { :success => "Thank you! This picture has been added to your Favorites List" }
    else
      redirect_to :action => 'index'
      flash[:success] = 'Thank you! This picture has been updated' 
    end
  else
    render 'edit'
  end
end

Here is my picture_param method in my PicturesController

 def picture_params
  params.require(:picture).permit(:title, :category, :genre, :stars)
end

Here is what the two new columns do

ratings_count: Calculates the number of times a picture has been rated
rating_total: Calculates the sum of the stars a picture has received

In the above code, I first set the ratings_count to 0 if the picture doesn't have a rating. This means that the picture hasn't been rated yet.

I then need to initially set the rating_total to the number of stars a picture has. If a user changed the star rating, I would add those stars to the rating_total. And if the total increased, that's my cue to increase the number of ratings.

Obviously, to calculate the average, I'd do something like this.

(@picture.rating_total / @picture.ratings_count).to_f

Now, I think I have the right idea but I know why this doesn't work. When columns are created with an integer value, by default they are set to nil. This leads to a NilClass Error when I load the web page.

undefined method `/' for nil:NilClass

Here is my code in the View

<li><strong>Rating:</strong> <%= pluralize((@picture.rating_total / @picture.ratings_count), 'Star') %></li>

Solution

  • Ok, the main reason it is not working is because

    • you fetch the picture
    • you check the stars from the database, and the NOT the passed form-parameters
    • you do update_attributes, which if I am not mistaken, used to set attributes and then save the complete object, but since rails 4 only updates the passed attributes (which is what you would expect)

    One small remark: keeping the rating correct is a function I would place in the model, NOT in the controller.

    Furthermore, how to handle the if nil, initialise to zero I wrote a short blogpost about. In short: overrule the getter.

    So I would propose the following solution. In your model write

    class Picture < ActiveRecord::Base
    
    
      def ratings_count
        self[:ratings_count] || 0
      end
    
      def ratings_total
        self[:ratings_total] || 0
      end
    
    
      def add_rating(rating)
        return if rating.nil? || rating == 0
    
        self.ratings_count += 1
        self.ratings_total += rating
        self.stars = self.ratings_total.to_f / self.ratings_count
        self.save
      end
    
      def rating
        return 0 if self.ratings_count == 0
        self.ratings_total.to_f / self.ratings_count
      end
    

    and then the code in your controller becomes much cleaner and readable:

    def update
      @picture = Picture.find(params[:id])
    
      stars = picture_params.delete(:stars)
    
      if @picture.update_attributes(picture_params)
        @picture.add_rating stars
        unless current_user.pictures.include?(@picture)
          current_user.pictures << @picture
          redirect_to @picture, :flash => { :success => "Thank you! This picture has been added to your Favorites List" }
        else
          redirect_to :action => 'index'
          flash[:success] = 'Thank you! This picture has been updated' 
        end
      else
        render 'edit'
      end
    end
    

    I first delete the :stars from the parameters, because I do not want to save those, I want to use those for the add_rating. I then try to update_attributes, which will fail if there are any failing validations, and if that is ok, I will add_rating which itself will handle nil or zero correctly. Well granted: I do not know how you handle a "non-rating" (nil? zero?). It is possible a rating of zero should be added, because it will add a rating, but most UI I know do not allow to select 0 as rating, so you might want to change the zero handling.