Permalink
Browse files

Revert "Merge pull request #12480 from iwiznia/master"

This reverts commit e5f5a83, reversing
changes made to d7567f3.
  • Loading branch information...
1 parent cc362fe commit 365110196afcf952bc22729d4467d579b708328f @jeremy jeremy committed Oct 11, 2013
@@ -1,10 +1,3 @@
-* Add `flatten` and `flatten!` methods to Duration objects.
-
- Example:
- Date.today + (1.month + 1.month).flatten == Date.today + 2.months
-
- *Ionatan Wiznia*
-
* `require_dependency` accepts objects that respond to `to_path`, in
particular `Pathname` instances.
@@ -81,18 +81,6 @@ def as_json(options = nil) #:nodoc:
to_i
end
- # Flattens all the +parts+ of the duration, and returns a
- # new duration object.
- def flatten
- Duration.new(@value, flatten_parts)
- end
-
- # Flattens all the +parts+ of this duration.
- def flatten!
- @parts = flatten_parts
- self
- end
-
protected
def sum(sign, time = ::Time.current) #:nodoc:
@@ -109,14 +97,6 @@ def sum(sign, time = ::Time.current) #:nodoc:
end
end
- def flatten_parts
- @parts.inject({}) do |result, (name, value)|
- result[name] ||= 0
- result[name] += value
- result
- end.to_a
- end
-
private
def method_missing(method, *args, &block) #:nodoc:
@@ -145,21 +145,6 @@ def test_to_json
assert_equal '172800', 2.days.to_json
end
- def test_flatten
- a = 2.months
- b = (1.month + 1.month).flatten
-
- assert_equal a.parts, b.parts
- end
-
- def test_flatten!
- a = (1.month + 1.month)
- b = a.flatten
- a.flatten!
-
- assert_equal a.parts, b.parts
- end
-
protected
def with_env_tz(new_tz = 'US/Eastern')
old_tz, ENV['TZ'] = ENV['TZ'], new_tz
@@ -3699,21 +3699,6 @@ Time.utc(1582, 10, 3) + 5.days
# => Mon Oct 18 00:00:00 UTC 1582
```
-When addinng or substracting durations, the resulting duration will be equivalent to subsequent calls to since or advance, so:
-
-```ruby
-Date.new(2013,1,28) + 1.month + 1.month
-# => Thu, 28 Mar 2013
-```
-
-If you want to add durations and then use them as just one call to since or advance, you can use the flatten or flatten! method:
-
-```ruby
-Date.new(2013,1,31) + (1.month + 1.month).flatten
-# => Sun, 31 Mar 2013
-```
-
-
Extensions to `File`
--------------------

6 comments on commit 3651101

Member

robin850 replied Oct 13, 2013

@jeremy : Sorry but could you point why this was reverted please ?

Owner

rafaelfranca replied Oct 13, 2013

Because it's hard to discover, hard to understand, and borrows vocabulary from Array that doesn't fit.

Member

robin850 replied Oct 13, 2013

Ok, thank you! :-)

Contributor

iwiznia replied Oct 23, 2013

@rafaelfranca If you don't like the name, I can change it to whatever sounds better to you, but don't remove the functionality altogether, it's trying to solve a real problem, encountered by different people (there are 3 pull requests related to this).
Since this isn't changing any existing behaviour (thus not breaking anything), I think adding 2 methods is ok, specially since if this methods are not present, the solution would be to access the 'parts' attribute on the duration object, which I think it's a bad idea since that is implementation details and the consumer of the api shouldn't know about.

Owner

rafaelfranca replied Oct 23, 2013

It is not about the name. It is about the fix itself. This fix will not solve the issue and I bet people will still open issues on the rails issue tracker about it and we would have to point to this new method.

This would happen because this method is not easy to find and neither easy to understand why I need to call it.

There no way to fix this issue without changing the way duration works. So we prefer to not add this method.

Contributor

iwiznia replied Oct 23, 2013

I get that is a little hidden, but I bet people would find it before opening an issue.
And the most important part is that it's impossible to get the functionality of this methods without knowing and accessing the implementation details of the Duration.
And the fact that it doesn't change how duration works is a good thing since it's a new functionality...

Please sign in to comment.