search

scala / scala-library-next

backwards-binary-compatible Scala standard library additions
Apache License 2.0
69 stars 17 forks source link

add Iterator#dropInPlace #65

Open NthPortal opened 3 years ago

NthPortal commented 3 years ago

.drop(...) is equivalent to calling .next() (up to) a given number of times, which is very useful. unfortunately, .drop(...) is not required to return the same instance (though the default implementation does), which means you need to assign it to a fresh val. Having to create a second binding for the iterator is ugly, and potentially error prone if you accidentally keep using the old Iterator instance. .dropInPlace(...) would be guaranteed to return this.type, allowing you to keep a single val/binding.

julienrf commented 3 years ago

The semantics of dropInPlace in mutable collections is to mutate the collection. I don’t think we should use the same name for something different.

Would it be possible to refine the return type of drop to be this.type, instead? Such a change couldn’t be performed in scala-library-next, of course, it would have to wait for the next major version of the scala-library.

NthPortal commented 3 years ago

is this not mutating the iterator, which is a collection(-ish)?

julienrf commented 3 years ago

True, but the way I see it is that an Iterator is an auxiliary that I can use to iterate over an actual collection, it’s not really a collection itself. So, if I write List(1, 2, 3).iterator.dropInPlace(2) I find it a little bit confusing that I can “dropInPlace” on the immutable collection List. Maybe it’s just me… :slightly_smiling_face:

Nevertheless, I still think that what we really want in the long-term is to fix the signature of drop in Iterator, not to have both drop and dropInPlace, right?

Jasper-M commented 3 years ago

Currently the rule for all methods except next and hasNext is "don't use the iterator after calling the method". I think it might be confusing to add methods that deviate from this rule.

NthPortal commented 3 years ago

I like Julien's suggestion, and I'm okay with having specific exceptions to the rule/tweaking the rule

Jasper-M commented 3 years ago

Then IMO it would be better to add a method with a name that stands out more, like dropInPlace. That way it's clearer when reading code that it's a method that's exempt from the standard Iterator rule.

Are we even sure that drop is always equivalent to calling next n times, for every kind of Iterator? I can imagine that some iterators can have other implementations.

NthPortal commented 3 years ago

it's not equivalent; it has the same contract as drop—that is, n elements or until empty