code duplication between index implementations about alexandrie HOT 3 CLOSED

I'd love to contribute to this, but I don't want to get carried away here before i get some input ;)

here's my proposal-

1. split the Index implementations into two components, a Repository object which manages the git repo, and a Tree object which manages the filesystem. This removes the code duplication, as the Tree object will be shared between both Index implementations.
2. If you think it's necessary, you could also split the Indexer trait - trait Indexer: Repo + Tree. While you're at it you may wish to rename the Indexer trait to Index (imperative), and rename Index to LocalIndex
3. use conditional compilation to support different strategies (if you need to!) and scrap the Index enum.
4. Consider creating a File object which handles deserialising and editing a single file in the index. The Tree object doesn't need to know how to parse a file, it just needs to know how to search them, and where to insert new ones.
5. add some unit tests and bench tests. This will give you the confidence to refactor, and to decide whether you really want to maintain two modules that do the same thing (if one is objectively better than the other).
6. the implementation is currently mostly synchronous, but it doesn't need to be. Would be fairly trivial to make the filesystem IO async.
7. move the whole thing into a separate crate. This could either be a separate repo, or at least a different crate in the same repo using a cargo workspace. The reason for this is that a well-implemented Index library would be a nice building block for other crates

of course all of this already exists in https://github.com/Rust-Bucket/Crate-Index. It's up to you which way you want to go with this. I don't really mind copying and pasting the best of that crate into this one.

from alexandrie.

Thanks a lot for the insights.

I completely agree that the code duplication in the indexes' implementations should be addressed.
I read what you did in #40 and in https://github.com/Rust-Bucket/Crate-Index and I like the implementations there, with the separation of Tree and Repository.
I am completely open to factor out the index management parts in its own crate, and https://github.com/Rust-Bucket/Crate-Index could be exactly that.

I also agree with moving to async filesystem IO and having an abstraction (like File, as you proposed) to abstract how to traverse and search crate records.
It is not clear to me if having the Repo and Tree traits (to make them super traits of Indexer) is necessary, I think we can try just having the Repository and Tree structs initially and see how to move from there.

For the naming thing of Index/Indexer, I also agree that the trait should (and should have always been) Index, but then I didn't know what to name the enum. So I made that weird naming scheme.
I also inclined to fix this, renaming Indexer to Index and the enum to LocalIndex, this name looks good to me.

from alexandrie.

As you can see, I'm keen to contribute! I think i'd better take a deep breath and slow down a bit though.
I'll submit a pull request to eliminate the code duplication for now, and everything else can come later, or never, as appropriate ;)

from alexandrie.