compenguy / ngrammatic

A rust crate providing fuzzy search/string matching using N-grams
MIT License
25 stars 7 forks source link

Make CorpusBuilder::fill accept more generic iterator #7

Closed mexus closed 2 years ago

mexus commented 2 years ago

Without the MR it is impossible to fill the CorpusBuilder with an iterator that yields Strings without having to allocate a storage.

Let me illustrate what I mean with some unit tests:

    #[test]
    fn accept_iterator_of_strings() {
        let provider = Vec::<String>::new().into_iter();
        // The test is only meant to verify that `fill` accepts an iterator that
        // yields `String`s.
        let _ = CorpusBuilder::new().fill(provider);  // Won't compile on "master".
    }

    #[test]
    fn accept_iterator_of_string_slices() {
        let provider = Vec::<String>::new();
        // The test is only meant to verify that `fill` accepts an iterator that
        // yields `&str`s or `&String`s.
        let _ = CorpusBuilder::new()
            .fill(&provider) // Impossible on "master"
            .fill(provider.iter().map(String::as_str));
    }

Hence, I propose this patch which makes the fill to be more generic and helps to avoid unnecessary allocations/conversions.

Technically this is a breaking change, since the 'a lifetime is removed from the fill's signature, but actual used code won't break, since types that implement Iterator<Item = &str> are accepted without a need to change anything.

P.S.

Thanks for the great library!!

compenguy commented 2 years ago

Thanks! I'll bump to 0.4 and make a new release shortly.

mexus commented 2 years ago

Many thanks for you blazing fast response! :))