how to perform "most viewed"

i have a poem model. i need to display the most viewed poems.
is there any plug inns out there to do this?

-nirosh-

nirosh wrote:

i have a poem model. i need to display the most viewed poems.
is there any plug inns out there to do this?

Unless I'm missing something obvious, I wouldn't think a plugin would be
needed for this.

poems_controller.rb

...or use update_counters
http://api.rubyonrails.org/classes/ActiveRecord/Base.html#M001792

Cheers,
Mohit.
20/8/2010 | 4:00 AM.

nirosh wrote:

i have a poem model. i need to display the most viewed poems.
is there any plug inns out there to do this?

Unless I'm missing something obvious, I wouldn't think a plugin would be
needed for this.

poems_controller.rb
--------------------
def show
@poem = Poem.find(params[:id])
@poem.increment!(:view_count)

Don't use increment. It won't do what you want since it's only incrementing the value of the object and then saving it. Not updating the value in the database. That is to say...

u = User.find(2)
u1 = User.find(2)
u.login_count

=> 7

u1.login_count

=> 7

u1.increment!(:login_count)
u1.login_count

=> 8

u.login_count

=> 7

u.increment!(:login_count)
User.find(2).login_count

=> 8

Oops!

You want to use increment_counter which calls update_counters which does this in the database:

UPDATE poem SET view_count = view_count + 1 WHERE id = 123

So... you want this:

def show
    Poem(:view_count, params[:id])
end

Don't use increment. It won't do what you want since it's only incrementing the value of the object and then saving it. Not updating the value in the database.

What is "saving" if not updating in the database?

You want to use increment_counter which calls update_counters which does this in the database:

What is the result of your "u" "u1" console experiment when you use
increment_counter rather than increment?
As far as I can see it will be the same - you've got two copies of an
object loaded, and you update one, without reloading the other; of
course that can lead to race conditions and stale data.

Don't use increment. It won't do what you want since it's only incrementing the value of the object and then saving it. Not updating the value in the database.

What is "saving" if not updating in the database?

Let's say we have User u with id 123. u.foo = 1.

u.increment(:foo) will execute this: update users set foo = 2 where id = 123

If I have multiple copies of User 123 lying around (different mongrels perhaps) there is a race condition. The longer I hold onto a copy of User 123 the better chance I have of losing the true value of foo since increment() will *overwrite* the value in the database, not add to it.

If however I do this...

User.increment_counter(:foo, 123) that will execute this: update users set foo = foo + 1 where id = 123

It doesn't matter how many times or where I call it, the *database* will contain the correct value of foo.

It's very possible that my local copy "u" won't until I reload it however.

You want to use increment_counter which calls update_counters which does this in the database:

What is the result of your "u" "u1" console experiment when you use
increment_counter rather than increment?
As far as I can see it will be the same - you've got two copies of an
object loaded, and you update one, without reloading the other; of
course that can lead to race conditions and stale data.

No. increment_counter doesn't operate on an instance of the class....

u = User.find(2)
u.login_count

=> 8

User.increment_counter(:login_count, 2)

  User Update (3.0ms) UPDATE "users" SET "login_count" = COALESCE("login_count", 0) + 1 WHERE ("id" = 2)
=> 1

u.login_count

=> 8

u.reload

  User Load (1.7ms) SELECT * FROM "users" WHERE ("users"."id" = 2)

u.login_count

=> 9

Ah! That's the bit that wasn't clear to me the way you phrased it
before. Reading your post again, I can see what you mean now.

Cheers,

increment() will *overwrite* the value in the database, not add to it.

Ah! That's the bit that wasn't clear to me the way you phrased it
before. Reading your post again, I can see what you mean now.

The annoying thing is that if your site doesn't get much traffic increment() will appear to work. It's only when there's concurrent access that it will start to break down.

This next part is for the archives...

As a general rule, if you are incrementing or decrementing fields that are counters, do not use any Rails method that *set* the value and save it. Make sure the method results in a "UPDATE table SET c = c + 1" type of query.

If you write your own implementation, be sure to consider the situation in which your database column is NULL.

And if you're doing a lot of this, you might find this useful http://github.com/phallstrom/set_counters as well. Or not :slight_smile:

-philip

Michael Pavling wrote:

Don't use increment. �It won't do what you want since it's only incrementing the value of the object and then saving it. �Not updating the value in the database.

What is "saving" if not updating in the database?

You want to use increment_counter which calls update_counters which does this in the database:

What is the result of your "u" "u1" console experiment when you use
increment_counter rather than increment?
As far as I can see it will be the same - you've got two copies of an
object loaded, and you update one, without reloading the other; of
course that can lead to race conditions and stale data.

Besides this. According the the Rails docs, increment_counter is
typically used for caching aggregate values. That's not the case here
since the field to be incremented is not related to an associated model.
The increment_counter would make sense for things like caching the count
of comments on a post model to prevent the SQL query that would be
required to count the associated comments.

And, of course, that race condition exists in your console experiment,
but that's not an issue in the controller method I presented earlier.
The @poem instance would be garbage collected at the end of the
request-response cycle. Other methods would have to reload from the
database anyway. So I don't see a problem using the increment! method
here.

Robert Walker wrote:

Besides this. According the the Rails docs, increment_counter is
typically used for caching aggregate values. That's not the case here
since the field to be incremented is not related to an associated model.
The increment_counter would make sense for things like caching the count
of comments on a post model to prevent the SQL query that would be
required to count the associated comments.

And, of course, that race condition exists in your console experiment,
but that's not an issue in the controller method I presented earlier.
The @poem instance would be garbage collected at the end of the
request-response cycle. Other methods would have to reload from the
database anyway. So I don't see a problem using the increment! method
here.

------------------------------
def show
  @poem = Poem.find(params[:id])
  @poem.increment!(:view_count)
  ...
  ...
end
------------------------------

Disregard this. I see the issue presented by Philip Hallstrom now. Makes
sense. Maybe we should consider deprecating increment! given the race
condition due to multi-threading/multi-processes.

And, of course, that race condition exists in your console experiment,
but that's not an issue in the controller method I presented earlier.

Yes, it is. What happens when two people hit the URL that triggers that action at the same time?

Change the action to this...

def show
@poem = Poem.find(params[:id])
sleep(rand(10)) # to simulate something that takes awhile
@poem.increment!(:view_count)
...
...
end

Let's say that view_count is 1 for the given poem. Hit that page with two browsers at the same time and when they both finish view_count will be 2. It should be 3.

The @poem instance would be garbage collected at the end of the
request-response cycle.

True, but even quick Rails processes take a little bit of time. As soon as there is *any* time between when you retrieve the record and when it gets incremented, you have a chance for a race condition because the increment (via increment!()) isn't incrementing. It's overwriting.

Robert Walker wrote:

Besides this. According the the Rails docs, increment_counter is
typically used for caching aggregate values. That's not the case here
since the field to be incremented is not related to an associated model.
The increment_counter would make sense for things like caching the count
of comments on a post model to prevent the SQL query that would be
required to count the associated comments.

And, of course, that race condition exists in your console experiment,
but that's not an issue in the controller method I presented earlier.
The @poem instance would be garbage collected at the end of the
request-response cycle. Other methods would have to reload from the
database anyway. So I don't see a problem using the increment! method
here.

------------------------------
def show
@poem = Poem.find(params[:id])
@poem.increment!(:view_count)
...
...
end
------------------------------

Disregard this.

Too late. I already responded :slight_smile:

I see the issue presented by Philip Hallstrom now. Makes
sense. Maybe we should consider deprecating increment! given the race
condition due to multi-threading/multi-processes.

Maybe. But then again it may have it's uses. Maybe you know what you're doing and really just want to update the model (assuming the non-bang version). I think it would be a good idea for the docs to mention the race condition and point people to increment_counter.

increment! could also simply hand off to increment_counter, but that might have some negative side affects for existing code that rely on increment/increment!.

I don't know....

-philip

Philip Hallstrom wrote:

And, of course, that race condition exists in your console experiment,
but that's not an issue in the controller method I presented earlier.

Yes, it is. What happens when two people hit the URL that triggers that
action at the same time?

Sorry I was completely forgetting about the very important rule for any
GET request (such as with the show action). Get request should be
idempotent!

http://en.wikipedia.org/wiki/Idempotent

As I realized early and posted to the effect. I do completely agree with
you.