Added Range type

[danieldietrich]

No description provided.

[codecov-io]

Codecov Report

Merging #2490 into v2.0.0 will decrease coverage by 9.02%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             v2.0.0    #2490      +/-   ##
============================================
- Coverage     90.69%   81.67%   -9.03%     
- Complexity      166      167       +1     
============================================
  Files            11       12       +1     
  Lines           344      382      +38     
  Branches         65       75      +10     
============================================
  Hits            312      312              
- Misses           32       70      +38

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/io/vavr/collection/Range.java	0% <0%> (ø)	0 <0> (?)
src/main/java/io/vavr/Iterable.java	40% <0%> (-60%)	2% <0%> (+1%)
src/main/java/io/vavr/control/Option.java	100% <0%> (ø)	43% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff10d9c...238e843. Read the comment docs.

[mincong-h]

Your solution is much less verbose than mine 👍

[mincong-h]

Do you consider removing some else?

if (step == 0) {
    throw new IllegalArgumentException("step cannot be 0");
}
if (from == toInclusive) {
    return () -> Iterator.of(from);
}
if (step > 0) {
    ...
} else {
    ...
}

[mincong-h]

Do we have all the existing tests running? I think I understand your approach, but it's very complex... Personally, I prefer not rely on inclusiveBy...

[mincong-h]

This is elegant, combined with the abstract hasNext() 👍

[mincong-h]

Do you think about using another implementation for step=1? The overflow detection can be avoided, which can improve performance, as I believe most people are using range without giving step (step=1). JDK does it without overflow detection with step=1, see java.util.stream.Streams.RangeIntSpliterator.

[ullenboom]

I really like a Range type. Some wishes and ideas:

a. Write Javadoc.

b. Store from, to and step into final variables. This has two advantages:

1. a proper toString() method can return all values and current state (useful for debugging)
2. introduce a message and pass into the constructor of throw new NoSuchElementException(msg); it is best practise to give as much context as possible.

c. Something controversial:

1. The default notation for ranges in Java is: start inclusive and end exclusive. Because of that, a method can be simplifies to Range.of(...).

2. A contains(...) method would be convenient if its intended to evolve the data type. Other methods would be convenient too, how about https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.ranges/-int-progression/index.html?

[danieldietrich]

@ullenboom Thanks for your suggestions. The PR is marked as Draft / WIP.

Write Javadoc.

This is part of our definition of done. You can expect something, otherwise the PR will not be pulled :)

Store from, to and step into final variables.

If you mean making Range a class instead of an interface: yes, I plan to do that. Probably I will align to Scala. See https://www.scala-lang.org/api/current/scala/collection/immutable/Range.html

The default notation for ranges in Java is (...)

Yes, in Scala an exclusive end is also the default. There the syntax is 1 until 10, which is syntactic sugar for Range(1, 10), which is syntactic sugar for Range.apply(1, 10). Indeed, we could use Range.of for that purpose in Vavr.

The inclusive end is denoted Range.inclusive(1, 10), which is the same as 1 to 10 there.

(Additionally, there are method overloads for the step parameters).

A contains(...) method

In Scala, Range inherits from IndexedSeq. It is a collection of elements, which makes sense to me.