SE-like voting system

Question

I've created a voting system that's similar to Stack Exchange's in a Ruby on Rails web app. It's working great - as far as I can tell, all the edge cases are caught. This is the action to cast a vote.

There are some conditions I've had to satisfy:

• must not allow duplicate voting, even from console requests
• one vote per post (can't have an up and a down vote on the same post)
• must be logged in

At the moment, I'm updating the record if there's already a vote by this user on this post, but I wonder if that's the most efficient strategy. Would destroying the old row and creating a new record be faster?

I'd particularly appreciate efficiency comments - at the moment, this is a very server heavy action, which I'd like to minimise.

def create
  post = (params[:post_type] == "a" ? Answer.find(params[:post_id]) : Question.find(params[:post_id]))
  existing = Vote.where(:post => post, :user => current_user)

  if existing.count > 0
    if existing.first.vote_type == params[:vote_type].to_i
      render :plain => "You have already voted.", :status => 409 and return
    else
      # There's already a vote by this user on this post, so we may as well update that instead of removing it.
      vote = existing.first
      vote.vote_type = params[:vote_type].to_i
      vote.save!

      state = { :status => "modified", :vote_id => vote.id }
      render :json => state and return
    end
  end

  vote = Vote.new
  vote.user = current_user
  vote.post = post
  vote.vote_type = params[:vote_type]
  vote.save!

  state = { :status => "OK", :vote_id => vote.id }
  render :json => state and return
end

(A quick note on vote types: the Vote.vote_type column has type integer, and the possible values are 0 (upvote) or 1 (downvote)).

Answer 1

First of all, I'd strongly encourage you to change your vote-scheme to use a value attribute of either 1 or -1, rather than a vote_type that can be 0 or 1.
A) because +1/-1 is what the votes actually mean, and B) because it makes it a heck of a lot easier to calculate the total. You just have to do:

score = some_post.votes.sum(:value)

Obviously, it'd still be a good idea to cache the score somewhere when appropriate. The database is fast, but retrieving a cache is faster.

In the following I've assumed it's set up like that, because it makes a ton more sense.

I would also recommend that you just have 1 post model; not an Answer and a Question as separate models. I'd wager the two are practically identical anyway, and it'd mean you don't have to find one or the other. You can just call Post.find(id) and done. For this especially, there's no difference between a question and an answer.

Second, you can use .find_or_initialize_by for this:

vote = post.votes.find_or_initialize_by(user: user)
if vote.value == params[:value].to_id
  # already voted
else
  # never voted (new vote record) or changing the vote
  vote.value = params[:value].to_id
  vote.save!
end

If you can skip loading the post, that'll save you some time too (however minor), since you don't actually need the post itself for anything in particular. You just need its ID for reference. Of course, it's safer to load the post, just so you know it exists (otherwise, I could preemptively up- or downvote future posts by passing a large post ID to the action). But even so, you can add a .select(:id) to the query, so you don't waste time loading all the attributes you don't need here and now - e.g. a large piece of text content or similar. And if you have eager-loading associations, nerf them. Again, you only need to post's ID.

Alternatively, you can set up a uniqueness constraint on the post_id/user_id columns, and - if your database supports it! - use a bit of raw SQL:

INSERT INTO votes (post_id, user_id, value) VALUES (?, ?, ?)
  ON DUPLICATE KEY UPDATE value = ?;

You'd want to safely insert the right values in the sql, maybe add values for created_/updated_at, etc..

The above would just insert a new row if the user hasn't voted before, but if he/she has, the constraint is triggered, and it performs an update on the existing row instead.

The major drawback is of course that you don't get any immediate feedback from the database about what happened. You can be sure that the database is correct, but you won't be able to send the 409 Conflict. And again, not all databases support this.

You can also use a uniqueness constraint to trigger a RecordNotUnique exception: Attempt to insert a new record, and if the exception is raised, rescue it and update instead. Sort a manual, Rails-side version of ON DUPLICATE KEY UPDATE.

In any case, I wouldn't worry too much about performance for this. A database is fast, and voting is relatively rare (a post is likely viewed many more times than votes are cast on it). You're probably spending 10x as much time in Rails code, as the database is spending doing all the selects, inserts and updates you throw at it.