Add Array#to_ranges

[aguynamedryan]

I needed this code for a project and figured I'd offer it up to facets

I originally found this code at
https://dzone.com/articles/convert-ruby-array-ranges
but it didn't actually work as described, so I fixed it.

[trans]

Looks good to me. But the tests didn't run for some reason. I don't think it's your code. I'll look into it and get back to you.

[ioquatix]

It's odd that travis blew up.

Can we make self.compact.uniq.sort better?

Compact is O(N). Uniq might be O(N^2) search? sort is okay.

However, it's also making 3 copies of the array for no reason. Could be even self.dup.sort!.uniq!.compact! or something like that.

[ioquatix]

if !array.empty? AFTER you already did compact.uniq.sort seems inefficient. It could be the first line.

return self if self.empty?

[trans]

Yes, but it should be a new array.

return [] if empty?

[ioquatix]

Well, it depends on the assumptions you want to make, but I prefer not doing additional allocations.

^_^ > irb
2.3.0 :001 > x = []
 => [] 
2.3.0 :002 > x.object_id
 => 70296120613840 
2.3.0 :003 > x.sort.object_id
 => 70296111721840 

As an example, sort returns a new array, so it supports your argument.

[trans]

In fact something like:

    def to_ranges
      ranges = []
      return ranges if empty?
      array = uniq.compact
      return ranges if array.empty?
      array.sort!
      ...

And get rid of the if !array.empty? clause altogether.

[ioquatix]

While we are on the topic of bike shedding lol, perhaps compact is unnecessary. Because it should be a contract of the caller to provide the right kind of array?

[ioquatix]

Anyway if it can be improved with the above advice I think it's a good addition.

[trans]

Well, it depends on the assumptions you want to make, but I prefer not doing additional allocations.

In this case you really have to because if you return self they could modify it in place,

f = [1,2,3]
g = f.to_ranges
g << (4..5)
f  => [1..3,4..5]

And that's not what would be expected.

[trans]

While we are on the topic of bike shedding lol, perhaps compact is unnecessary. Because it should be a contract of the caller to provide the right kind of array?

I suspect that the algorithm won't do what one expects then. I have to try it, but I expect we'd get things like,

[1,2,2,3,3].to_ranges  #=> [1..2,2..2,3..3]

And I don't think that would ever be what anyone wants. That being so the compact is really an integral part of the behavior.

@aguynamedryan What do you think?

[trans]

Hey I just rediscovered that Facets already has this, arrange. So the task now becomes to compare implementations, and improve the code accordingly. Also, I think we need to ask if the name "arrange" is really the right one. Perhaps at the very least, we could alias to_ranges -- but is it good practice to call a method "to_foo" when there is no Foo class?

[ioquatix]

Well, there is a Range class. to_ranges is just a plural.. so it makes sense in a way.

[ioquatix]

The name arrange is pretty unintuitive IMHO.

[trans]

@ioquatix You know, you are right. I'll change it to #to_ranges and figure out the best implementation between this and the current #arrange.

Someone remind me to do this if I haven't gotten around to it within the next few weeks.