This project is archived and is in readonly mode.

#5794 ✓invalid
eydaimon

using include? on a time range yields unhappy results

Reported by eydaimon | October 12th, 2010 @ 09:07 PM | in 3.x

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.

Comments and changes to this ticket

  • Aditya Sanghi

    Aditya Sanghi October 12th, 2010 @ 10:47 PM

    • 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.

  • eydaimon

    eydaimon October 12th, 2010 @ 11:17 PM

    • Tag changed from rails3.0.0, range, time to 1.9.2, rails3.0.0, range, time
  • Aditya Sanghi

    Aditya Sanghi October 12th, 2010 @ 11:33 PM

    • 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.

  • Aditya Sanghi
  • Aditya Sanghi

    Aditya Sanghi October 18th, 2010 @ 03:42 PM

    @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.

  • Szymon Nowak

    Szymon Nowak October 18th, 2010 @ 07:44 PM

    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

  • Aditya Sanghi

    Aditya Sanghi October 19th, 2010 @ 11:13 AM

    • 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.

  • eydaimon

    eydaimon October 19th, 2010 @ 04:34 PM

    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.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages