Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Iterator#dropInPlace #65

Open
NthPortal opened this issue Feb 9, 2021 · 7 comments
Open

add Iterator#dropInPlace #65

NthPortal opened this issue Feb 9, 2021 · 7 comments
Labels
enhancement New feature or request library:collections status:pending This enhancement request lacks consensus on whether or not to include it

Comments

@NthPortal
Copy link
Contributor

.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 NthPortal added enhancement New feature or request library:collections status:pending This enhancement request lacks consensus on whether or not to include it labels Feb 9, 2021
@julienrf
Copy link
Contributor

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
Copy link
Contributor Author

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

@julienrf
Copy link
Contributor

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… 🙂

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
Copy link
Member

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
Copy link
Contributor Author

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

@Jasper-M
Copy link
Member

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request library:collections status:pending This enhancement request lacks consensus on whether or not to include it
Projects
None yet
Development

No branches or pull requests

3 participants