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

Collection[WithId]::new() could take any kind of iterable instead of a Vec #485

Closed
woshilapin opened this issue Nov 28, 2019 · 3 comments
Closed

Comments

@woshilapin
Copy link
Contributor

Today, Collection::new() and CollectionWithId::new() takes a Vec<T: Id<T>> as an input argument. But technically, nothing should prevent us to construct a Collection or a CollectionWithId from a HashSet or a HashMap::values().

This would mean change the CollectionWithId::new() method from (and similar for Collection)

pub fn new(v: Vec<T>) -> Result<Self>;

to something like

pub fn new(v: I) -> Result<Self>
where
	I: IntoIterator<Item = T>;
@woshilapin
Copy link
Contributor Author

I experimented this implementation but here are the problems.

If the new method is changed to

pub fn new(v: I) -> Result<Self>
where
	I: IntoIterator<Item = T>;

Then the implementation will call .into_iter() then .collect() to collect as a Vec. If the input parameter is already a Vec, this is an aberration to iterate over it in order to collect it again as a Vec.

Another solution would be to provide a From<IntoIterator<Item = T>> implementation. But this one causes also a problem. Below is a possible implementation.

impl<T: Id<T>, I: IntoIterator<Item = T>> From<I> for CollectionWithId<T> {
    fn from(iterator: I) -> Self {
        // This cannot fail since there will be a unique identifier in the
        // collection hence no identifier's collision.
        CollectionWithId::new(iterator.into_iter().collect()).unwrap()
    }
}

But it does not compile since it creates a conflicting implementation.

error[E0119]: conflicting implementations of trait `std::convert::From<collection::CollectionWithId<_>>` for type `collection::CollectionWithId<_>`:
   --> collection/src/collection.rs:405:1
    |
405 | impl<T: Id<T>, I: IntoIterator<Item = T>> From<I> for CollectionWithId<T> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: conflicting implementation in crate `core`:
            - impl<T> std::convert::From<T> for T;

error: aborting due to previous error

Anyone has some ideas about that?

@woshilapin
Copy link
Contributor Author

Note that there is a FromIterator trait that could be implemented but this option has already been explored and rejected in #291.

@woshilapin
Copy link
Contributor Author

Therefore, let's close this issue (especially since this functionality has moved to its own repository now hove-io/typed_index_collections).

@woshilapin woshilapin closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant