+1 to matching Redis-style INCR. Returning `nil` is useful in some cases, like lazy-initializing a counter to a value that's expensive to calculate, but for typical use it's a strange hassle.
Note that Memcached doesn't support negative counters, so `@data.decr(normalize_key(name, options), amount, options[:expires_in], -amount)` may have unexpected results (setting to 0). On Monday, September 25, 2017 at 11:40:13 PM UTC-7, Aero Astro wrote: > > Hi there. > > I made a Pull Request to fix bug on > ActiveSupport::Cache::MemCacheStore#increment > and decrement for initialization of not stored value. > > https://github.com/rails/rails/pull/30716 > > > Regarding initialization, the current document says as follows. > > ``` > # Increment a cached value. This method uses the memcached incr atomic > # operator and can only be used on values written with the :raw option. > # Calling it on a value not stored with :raw will initialize that value > # to zero. > ``` > > However, I would rather initialize the value with `amount` for increment > and `- amount` for decrement > as if the not stored value is initialized to 0 prior to the specified > operation. > > ``` > # Before > @data.incr(normalize_key(name, options), amount, options[:expires_in], 0) > @data.decr(normalize_key(name, options), amount, options[:expires_in], 0) > > # After > @data.incr(normalize_key(name, options), amount, options[:expires_in], > amount) > @data.decr(normalize_key(name, options), amount, options[:expires_in], > -amount) > ``` > > > The reason is that we want to implement appropriate lazy initialization. > For example, simple counter with lazy initialization can be written with > instance variable as follows. > > ``` > def increment(amount) > @counter ||= 0 > @counter += amount > end > ``` > > If we implement the above counter with current > ActiveSupport::Cache::MemCacheStore, it can be written as follows. > > ``` > def increment(amount) > result = @cache.increment(amount) > result != 0 ? result : @cache.increment(amount) > end > ``` > > As you can see, the initialization to 0 inside @cache actually does not > help us write lazy initialization. > We should still handle 0 instead of nil. What is worse, if we use > increment and decrement concurrently > on the same key, there is no way to calculate the accurate count. > > With the proposed feature request the above code can be simplified as > follows. > > ``` > def increment(amount) > @cache.increment(amount) > end > ``` > > Also, I have found the same specification on incr of Redis. > https://redis.io/commands/incr > > If the idea is O.K., I will submit another PR or change the current PR to > address the issue. > IMHO, as this initialization has been disabled for a long time, it is > about time to improve the specification. > > -- > Best Regards, > Takumasa Ochi > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at https://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
