Skip to content

usability issue: connect()-like functionality is only available on slices, not on Iterator(Item=Str) #22754

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

Closed
Byron opened this issue Feb 24, 2015 · 12 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Byron
Copy link
Member

Byron commented Feb 24, 2015

In other languages, this kind of functionality is easily achieved:

# my_fields can be an iterator
", ".join(my_fields)

In Rust, this currently only works on slices and vectors. However, in generic code, one usually has an iterator only, and I believe it should be simple to connect such an iterator if the item type is compatible.

Otherwise I fear people will start to throw in an intermediate collect::<Vec<String>>() call, which has performance implications.

All details, code, and the current workaround using itertools.intersperse() can be found in the respective question on stackoverflow.

Please note that this seems to be related to a more general issue, namely that some useful functionality is not implemented on Iterator, even though it could be: http://internals.rust-lang.org/t/add-dedup-for-iterator/1567 .

Meta

rustc 1.0.0-nightly (522d09d 2015-02-19) (built 2015-02-19)
binary: rustc
commit-hash: 522d09d
commit-date: 2015-02-19
build-date: 2015-02-19
host: x86_64-apple-darwin
release: 1.0.0-nightly

@tbu-
Copy link
Contributor

tbu- commented Feb 24, 2015

In Python this is basically just implemented on the equivalent of Vecs, if you pass in an iterator, the function first collects these elements into a vector and then joins them.

@Byron
Copy link
Member Author

Byron commented Feb 24, 2015

I think the point is that one can pass an iterable, whereas in Rust this is currently not possible. Also there seems to be no reason to not have such functionality readily available. Of course I might be totally mistaken there given my greenling status with Rust. Nonetheless I try to point out a usability issue, and addressing it will make using Rust easier in the given context.

@tbu-
Copy link
Contributor

tbu- commented Feb 24, 2015

I just addressed your

Otherwise I fear people will start to throw in an intermediate collect::<Vec<String>>() call, which has performance implications.

by pointing out that Python does that exact thing in its "connect" (join) function.

@Byron
Copy link
Member Author

Byron commented Feb 24, 2015

I think it would be comparable if one would have to write something like this: ", ".join(list(my_iterator)). As far as I am concerned, if I don't know what it does, I won't care as long as it is comfortable to use.

Also I hope what I wrote makes at least some sense, I don't know if I this issue is complying with requirements, or if it is at the right spot even.

@bluss
Copy link
Member

bluss commented Feb 25, 2015

  1. .dedup() for an iterator is super simple to add by third parties (see itertools), while Vec's .dedup() is much more involved and requires unsafe code and concern for exception safety.

  2. .connect() and .concat() are available on Vec I think because: We can do it "perfectly". We allocate the exact space needed beforehand, and do the concatenation. An iterator implementation has this imperfection that we can't know how much memory to allocate beforehand -- this has a certain performance cost. It's almost Rust tradition to punt the issue until it can be resolved in the best way.

And note, the string-joining functionality on iterators was implemented by a trait in collect-rs. I've imported it into itertools, so at least it's available there.

@tbu-
Copy link
Contributor

tbu- commented Feb 25, 2015

@bluss Shouldn't connect and concat be implemented on slices though?

@Byron
Copy link
Member Author

Byron commented Feb 25, 2015

Thank you - the newly added join() implementation will work for me.

@bluss About 2) Isn't there a rewindable iterator Trait somewhere, which would allow to traverse sequences multiple times. In that case, one could have one run to compute the required string size, and a second one to fill in the result string.

@tbu-
Copy link
Contributor

tbu- commented Feb 25, 2015

@Byron That would probably be a Clone bound. If you can clone the iterator before traversing it for collection, you can call .count() on it.

@Byron
Copy link
Member Author

Byron commented Feb 25, 2015

True ! It could be that easy ! With that in mind, maybe connect() could actually make it into the IteratorExt Trait, as an optimal implementation would be possible (?).

@steveklabnik
Copy link
Member

Triage: no change here. The itertools crate has this.

@tbu-
Copy link
Contributor

tbu- commented May 25, 2016

Now that a crate implements this, it could be considered fixed. Adding it to the standard library would likely require an RFC.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 22, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 17, 2017

I agree with what has been written -- the itertools crate provides this along with lots of other iterator-related functionality that is less universally applicable than the iterator functionality provided in the standard library. If someone feels strongly that something from itertools should be provided in the standard library, that would need to be justified in an RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants