Changed the Range's step to Number instead of int

[l0rinc]

Until now a Range could only step by an integer value, even if its elements had other types, e.g. doubles.
This made stepping over ranges like 0.1..0.9strange, as that resulted in a single element.

Also, stepping over ObjectRange instances (like the above one) called the next/previous method reflectively, even if the range had a numeric type (the most common case).
This made it ~10x slower than e.g. IntRange.

The new implementation changed the step type to Number and unified the way stepping is done in
ObjectRange and IntRange, making them as fast as the original IntRange for numeric cases.

Also, IntRange treats the overflow issue by storing the intermediary value in a Long directly.

At the boundaries, the next and previous methods may now wrap to the other extreme - or return null.

---

I tested the speed via the following trivial method:

void testSpeed() {
    for (def i = 0; i < 10; i++)
        run()
}

private void run() {
    def time = System.currentTimeMillis()
    def size = 0G
    for (def i = 0; i < 100; i++) { // changed to `i = 0G` when testing ObjectRange
        for (def j = 0; j < 100; j++) {
            def range = i..j
            def inverseRange = j..i
            for (def k = 1; k < 100; k++) {
                size += range.step(k).size()
                size += range.step(-k).size()

                size += inverseRange.step(k).size()
                size += inverseRange.step(-k).size()
            }
        }
    }
    println "size = ${size} in ${(System.currentTimeMillis() - time) / 1000.0}s"
}

resulting in

* Old impl for ObjectRange: `66.30s`
* New impl for ObjectRange: `5.52s` (12x faster)

and

* Old impl for IntRange: `4.54s`
* New impl for IntRange: `4.52s` (same speed as before)

[l0rinc]

Should play well with #109

[blackdrag]

your change currently has two obvious problems. (1) returning Collections.emptyList() is only ok, if you say, that the result cannot be modified. The old implementation did not have this. You even had to change a test for this. (2) You changed step(int) to step(Number). This is not binary compatible. Java programs using Range directly will have to recompile. They will do so just fine, since Java will do the boxing, but an old library will not work with these new Ranges

[l0rinc]

Hello @blackdrag,

1. I will document it, if you think it's necessary, though I think it was already specified that it returned an empty list - and not that it returned a new list.
   Also, why would people modify a list, returned by an empty range?

2. You're right, I will add a delegate method (deprecating it would be weird, as int is probably the most common usage).

[blackdrag]

An empty list can still be a list that gets elements added. One more thing... can you show me the benchmark code you did for ObjectRange as well?

[l0rinc]

Yes, I changed the description to immutable empty list.

The ObjectRange is described in the above message in the comment: // changed to i = 0G when testing ObjectRange

[PascalSchumacher]

@paplorinc Your changes crash the build http://ci.groovy-lang.org/viewLog.html?buildId=27049&tab=buildResultsDiv&buildTypeId=Groovy_Jdk7Build

[l0rinc]

Thanks, will rerun it locally (can't view the TC build).
Could we enable CI trigger for pull requests also, as it's done for other OS projects (e.g. google/guava#2164 (comment))?

[PascalSchumacher]

@paplorinc You can view the tc build if you login as "guest" with a blank password.

CI trigger for pull request would be very helpful (we had this before the move to apache), but I was told it's not possible with teamcity and the read-only apache github mirror.

[l0rinc]

Thanks, the problem was, that in case of enums, at the boundaries, the previous and next methods have wrapped to the other extreme.
I.e. next for the last element returned the first one (I didn't know this was legal behavior).
Added a separate test case for this and for null values also.

[l0rinc]

Tests passed: 9696, ignored: 22
http://ci.groovy-lang.org/viewLog.html?buildId=27169&tab=buildResultsDiv&buildTypeId=Groovy_Jdk7Build

[tkruse]

Since #143 was merged, this PR should be rebased, and some conflicts can be expected.

[l0rinc]

Thanks, but I will rather abandon it at this stage.

[tkruse]

Possibly at least the increment/decrement optimization for numbers could be kept

[paulk-asert]

I copied out a few of the additional tests you added to cover existing functionality. Also, PRs #142 and #143 have been merged recently. It would be nice to incorporate a reworked version of this PR catering for those changes. If no-one else jumps in I'll take a look myself. Created this issue for tracking purposes: GROOVY-7877.

[l0rinc]

I will rebase it, seeing that there is need for it :)

[paulk-asert]

Just for fun, I had a look at what a utility method outside of Range might look like:
#359
I am wondering whether that might limit added complexity to the Range class? I'd be interested in what others think.

[l0rinc]

Revived in #360, please review.
Is it ok that the new Gradle 3 crashes the build on the java 6 Travis node?