Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

using include? on a time range yields unhappy results

#5794

Example:

Loading development environment (Rails 3.0.0)
ruby-1.9.2-p0 > (1.day.ago..Time.now).include? 3.hours.ago
   succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
/Users/daniel/.rvm/gems/ruby-1.9.2-p0/gems/activesupport-3.0.0/lib/active_support/time_with_zone.rb:316: warning: Time#
succ is obsolete; use time + 1
..... ad infinitum

solution is obviously to convert each item into an integer.

Reported by eydaimon · October 12th, 2010 @ 09:07 PM

State: invalid
Milestone: 3.x
Assigned to: nobody
Importance: Low

Activity

  1. Aditya Sanghi
    Aditya Sanghi
    • Importance changed from to Low

    Just to confirm, this is a problem only in 1.9.2, on 1.8.7 this code runs just fine.

    October 12th, 2010 @ 10:47 PM

  2. eydaimon
    eydaimon
    • Tag changed from rails3.0.0, range, time to 1.9.2, rails3.0.0, range, time

    October 12th, 2010 @ 11:17 PM

  3. Aditya Sanghi
    Aditya Sanghi
    • State changed from new to incomplete
    • Milestone set to 3.x
    • Assigned user set to Santiago Pastorino
    • Tag changed from 1.9.2, rails3.0.0, range, time to rails3.0.0, range, time

    I confirm that this indeed is a problem on 1.9.2.

    @eydaimon, feel free to submit patches.

    You may want to poke around in -

    activesupport/lib/active_support/core_ext/range/include_range.rb

    We would also need tests for this added of course.

    Marking this incomplete, until we have patches.

    October 12th, 2010 @ 11:33 PM

  4. Aditya Sanghi
  5. Aditya Sanghi
    Aditya Sanghi

    @eydaimon

    I reexamined your request and read this thread again and looked a bit harder at the issue.

    The "ad infinitum" is not really upto infinity its actually stepping thru EACH second between the upper bound and lower bound of your range. So even if this works, i would imagine it would be horrible in terms of performance. You get a sense of infinity because there is an error message with each step (a warning).

    Now in Ruby 1.9.2 Range's include? calls succ on each element of the range. Outside of Rails, iteration over time is not possible but in Rails we have TimeWithZone with does seem to allow it.

    One way to fix it would be to add the succ method in TimeWithZone class.

    activesupport/lib/time_with_zone.rb

      def succ
        self.+(1)
      end
    

    I added this in my local install of ruby and i got the expected result like you had in your original post.

    I need to probably dig deeper as to if this is really necessary or how else to solve this problem and write tests too.

    October 18th, 2010 @ 03:42 PM

  6. Szymon Nowak
    Szymon Nowak

    I don't really think it's Rails issue. Just use Range#cover? instead:

    (1.day.ago..Time.now).cover? 3.hours.ago
    => true
    

    However, it's Ruby 1.9 only. If you're still on 1.8 you can simply alias cover? to include? and use cover? everywhere. Maybe Rails could do it for you...

    Here's a bit more about it: http://rhnh.net/2009/08/03/range-include-in-ruby-1-9

    October 18th, 2010 @ 07:44 PM

  7. Aditya Sanghi
    Aditya Sanghi
    • State changed from incomplete to invalid
    • Tag changed from rails3.0.0, range, time to 1.9.2, rails3.0.0, range, time

    Thanks Szymon! Didnt know about the 1.9 changes.

    So the short of it is use cover? in 1.9 instead of include? and you should be good to go.

    Apart from your own code, if you see anywhere in Rails code where this is happening, please let us know and we'll reopen this ticket. Marking invalid for now.

    October 19th, 2010 @ 11:13 AM

  8. eydaimon
    eydaimon

    I said from the beginning the solution was just to use .to_i on the date fields. I don't see how it's invalid. It's unexpected behavior.

    October 19th, 2010 @ 04:34 PM