I’d put a method in my controller to increment the counter. It might look something like
However, a more idiomatic way in Ruby is to do
def increment_count if session[:counter].nil? session[:counter] = 0 end session[:counter] += 1 end
def increment_count session[:counter] ||= 0 session[:counter] += 1 end
(Yes, it can be done in one line, using a semi-colon as a delimiter, but I think the above is clearer)Then, I’d set an instance variable to this value in each of the actions I wanted to count
def index # ... @count = increment_count end
that would sum all the counted actions though, wouldn’t it – as the session :count variable can’t differentiate between actions. so if you’d viewed the index action 3 times and the add_to_cart action twice, the count would show as 5.
No Mark, it wouldn’t. Dave has placed the call to increment_count inside the index action and not the add_to_cart action, the result is increment_count is only called when the index action has been called.
So would doing something like this be ‘wrong’?
def index @products = Product.find_products_for_sale @date = Date.today @time = Time.now #initialize if session[:counter].nil? session[:counter] = 0 else session[:counter] += 1 end end
There are two issues with that code. The main problem is that if someone views the index for the first time, their count will be 0 instead of 1. You can either remedy this by setting the counter variable to 1 instead of 0, or pull the counter incremental outside of the if/else construct.
The second issue, which is more minor, is that it’s one of those bits of code that feels like it should be pulled out into its own private method. If you ever decide to expand it to another action, then pulling it out will make the work nice and DRY. (Of course, you could refactor when the need arises) I’m pretty poor at explaining myself on why it would be separate, and in reality, it could be contained within the index action, but my general rule of thumb is that if I think I might be served by making a method out of it, then I always do.
Benjamin asserts:This is what I tried, and it works pretty well. I understand the appeal of creating another function to handle the counter setting, but I prefer not to when we’re only using it once. If we used the counter functionality on other pages, I would probably abstract it like mentioned above.
The first line of the action takes advantage of assignment chaining. It sets session[:counter] to 0 if session[:counter] doesn’t exist (is equal to nil). It then adds one to session[:counter] and sets @count to that new value.
def index session[:counter] ||= 0 @count = session[:counter] += 1 @products = Product.find_products_for_sale end
Keep in mind there is still a required reset later on in the add_to_cart action, but this takes care of the initialization and increment.
Personally, I like to extract the method (I called mine
increment_index_access_counter) to simply reveal the intention of the code and hide the pure technicality of having to initialize the session counter. I think it reads better when you avoid mixing business objects (product), with somewhat technical aspects (session[:counter]) in the same method.
What I did was pretty simple, I added an increment_counter function, a protected one with the same code mentioned here.
def increment_counter if session[:counter].nil? session[:counter] = 0 end session[:counter] += 1 end
In the index action, I called the increment_counter, like this
In the view I use this code
def index increment_counter ...
<%= pluralize session[:counter], 'time' %>
Since the view can access the session variable, There’s no need to make an instance variable, or should we?