scala-library-next icon indicating copy to clipboard operation
scala-library-next copied to clipboard

add Iterator#dropInPlace

Open NthPortal opened this issue 4 years ago • 7 comments

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

NthPortal avatar Feb 09 '21 20:02 NthPortal

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.

julienrf avatar Feb 10 '21 07:02 julienrf

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

NthPortal avatar Feb 10 '21 08:02 NthPortal

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?

julienrf avatar Feb 10 '21 08:02 julienrf

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.

Jasper-M avatar Mar 26 '21 09:03 Jasper-M

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

NthPortal avatar Mar 26 '21 12:03 NthPortal

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.

Jasper-M avatar Mar 26 '21 18:03 Jasper-M

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

NthPortal avatar Mar 27 '21 09:03 NthPortal