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

Please provide a FromIterator impl #86

Open
joshtriplett opened this issue Oct 31, 2019 · 3 comments
Open

Please provide a FromIterator impl #86

joshtriplett opened this issue Oct 31, 2019 · 3 comments
Labels

Comments

@joshtriplett
Copy link

fst has all the necessary infrastructure for FromIterator to work; please consider implementing it, so that collect works, and in particular to make it easy to collect into Result<Fst, ...> or Result<Set, ...>.

Thank you!

@BurntSushi
Copy link
Owner

It's not clear to me that this is a good idea. Is there any precedent for implementing FromIterator for fallible operations? As an example, Set::from_iter already exists, but note that it returns a Result. That is, you cannot construct a Set from any iterator. The keys must be lexicographically sorted. So as far as I can see, there are only two possible implementations for FromIterator:

  • Consume the iterator into memory, sort the keys and then build the set.
  • Panic if the iterator yields an unsorted key.

Neither option seems particularly good to me.

@joshtriplett
Copy link
Author

Yes, there's precedent for fallible FromIterator; that's what allows .collect::<Result<Vec<_>, SomeError>().

@BurntSushi
Copy link
Owner

Sure, but that only works because of a blanket impl on Result itself. Which means you can't implement FromIterator<Vec<u8>> for Result<Fst<Vec<u8>>, fst::Error>. Unless I'm missing something. If so, can you show me how? I don't see it.

Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e997d571f41f88f3eba30660ee7b4429

Do you know of any FromIterator impls that are themselves fallible? I can't think of any off the top of my head in std. And a quick glance of the impls of FromIterator in std doesn't seem to show any. For example, if fallible FromIterator impls were allowed, then I'd expect to see a FromIterator<u8> for String, but I don't think such a thing can exist (without panicking or doing lossy decoding).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants