aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport/lib/active_support
diff options
context:
space:
mode:
authorDavid Chelimsky <dchelimsky@gmail.com>2013-08-12 19:40:11 +0200
committerDavid Chelimsky <dchelimsky@gmail.com>2013-08-12 19:55:15 +0200
commitaf3ea544dd670409e80585329b019f598d29cef5 (patch)
treed7e13a728dd4381e13a9612baf1816397b17223e /activesupport/lib/active_support
parentf948814b44e56643381f49ef0567fb44148f4efe (diff)
downloadrails-af3ea544dd670409e80585329b019f598d29cef5.tar.gz
rails-af3ea544dd670409e80585329b019f598d29cef5.tar.bz2
rails-af3ea544dd670409e80585329b019f598d29cef5.zip
Refactor Duration#inspect
In preparing https://github.com/rails/rails/pull/11855, it took me a minute to understand what was going on due to naming (parts refers first to an attr_accessor, then to a local, and is then reassigned), but also because the iterator conditionally builds nulls and then removes them. I refactored to something much more functional-looking that I find easier to read, but you may or may not. If you do, great! Enjoy! If not, oh well, I tried. Can't win 'em all :) Rationale: * no name conflict between local var and attr_accessor * no reassignment of local var * algorithm spelled out in steps * unused items in initial list filtered out early * empty-list case handled early instead of reassigning local var * no duplication of formatting strings ("0 seconds") Benchmarks (after PR #11855 merged): 10000.times do 1.second.inspect end original #inspect 0.350000 0.000000 0.350000 ( 0.354709) 0.330000 0.000000 0.330000 ( 0.331885) 0.330000 0.000000 0.330000 ( 0.334441) refactored #inspect 0.340000 0.000000 0.340000 ( 0.340080) 0.340000 0.010000 0.350000 ( 0.345069) 0.330000 0.000000 0.330000 ( 0.335873) 10000.times do (1.day + 1.month + 2.minutes + 1.day).inspect end original #inspect 0.400000 0.000000 0.400000 ( 0.403027) 0.400000 0.000000 0.400000 ( 0.403781) 0.390000 0.000000 0.390000 ( 0.387596) refactored #inspect 0.400000 0.010000 0.410000 ( 0.399792) 0.400000 0.000000 0.400000 ( 0.404145) 0.400000 0.000000 0.400000 ( 0.403820)
Diffstat (limited to 'activesupport/lib/active_support')
-rw-r--r--activesupport/lib/active_support/duration.rb14
1 files changed, 7 insertions, 7 deletions
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index f7b12a1db6..00727ef73c 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -70,13 +70,13 @@ module ActiveSupport
alias :until :ago
def inspect #:nodoc:
- consolidated = parts.inject(::Hash.new(0)) { |h,(l,r)| h[l] += r; h }
- parts = [:years, :months, :days, :minutes, :seconds].map do |length|
- n = consolidated[length]
- "#{n} #{n == 1 ? length.to_s.chop : length.to_s}" if n.nonzero?
- end.compact
- parts = ["0 seconds"] if parts.empty?
- parts.to_sentence(:locale => :en)
+ val_for = parts.inject(::Hash.new(0)) { |h,(l,r)| h[l] += r; h }
+ [:years, :months, :days, :minutes, :seconds].
+ select {|unit| val_for[unit].nonzero?}.
+ tap {|units| units << :seconds if units.empty?}.
+ map {|unit| [unit, val_for[unit]]}.
+ map {|unit, val| "#{val} #{val == 1 ? unit.to_s.chop : unit.to_s}"}.
+ to_sentence(:locale => :en)
end
def as_json(options = nil) #:nodoc: