mdsteele / rust-cab

Rust library for encoding/decoding Windows cabinet (.cab) files
MIT License
16 stars 9 forks source link

API Refactoring #24

Open ikrivosheev opened 1 year ago

ikrivosheev commented 1 year ago

@mdsteele hello! Thank you for the library! I am using it in my projects and wanna improve performance and API.

Now API for read Cabinet file is:

impl Cabinet {
    pub fn folder_entries(&self) -> FolderEntries<'_> {...}
    pub fn get_file_entry(&self, name: &str) -> Option<&FileEntry> {...}
    pub fn read_file(&self, name: &str) -> Result<FileReader<'_, R>> {...}
}

The biggest problem: I cannot store FolderReader state between unpacking two different file and I must load all needed DataBlock for every file. I want to add iterator over FileReader.

I have some ideas how to improve API:

First

impl Cabinet {
    // Old API
    pub fn folder_entries(&self) -> FolderEntries<'_> {...}
    pub fn read_file(&self, name: &str) -> Result<FileReader<'_, R>> {...}

    // Drop this API
    // pub fn get_file_entry(&self, name: &str)

    // Iterator over folders. And in FoldersReader we can iterator over FileReader
    pub fn folder_reader_iter(&self) -> FoldersReader
}

Second

impl Cabinet {
    // Old API
    pub fn folder_entries(&self) -> FolderEntries<'_> {...}
    pub fn read_file(&self, name: &str) -> Result<FileReader<'_, R>> {...}

    // Drop this API
    // pub fn get_file_entry(&self, name: &str)

    // Iterator over FileReader without intermediate iterator over folders.
    pub fn file_reader_iter(&self) -> FilesReader 
}

What do you think? I have both implementation)

mdsteele commented 1 year ago

Is it possible to have both methods at once? (folder_reader_iter and file_reader_iter). Then the caller could choose whichever is more convenient. Perhaps file_reader_iter could be implemented in terms of folder_reader_iter?

If it's not feasible to support both, then I don't think I have a strong preference between the two. Although I guess I might lean slightly towards the folder_reader_iter version, since it preserves information about the folder structure, which some callers might want. What do you think?