rust-embedded-community / embedded-sdmmc-rs

A SD/MMC library with FAT16/FAT32 support, suitable for Embedded Rust systems
Apache License 2.0
309 stars 73 forks source link

Make Directory::iterate_dir version returning an Iterator #125

Open orsinium opened 6 months ago

orsinium commented 6 months ago

In the current implementation, Directory::iterate_dir accepts a callback to be executed for each entry. Returning an Iterator instead could make working with it much easier:

  1. It would match std::fs::read_dir behavior.
  2. It would make it possible to stop the iteration at any point.
  3. It's easy to convert iterators to callbacks (using map method)
  4. It's hard to convert a callback into an iterator. Well, I don't currently see an easy way except pushing all entries from the callback into a vector, but that will require memory allocations.

Another well-known project to use iterators is embedded_graphics which proves that iterators are fast enough for use on embedded devices.

thejpster commented 6 months ago

I've used iterators widely so I don't think that was my reason for using a callback here. But without studying the code I can't remember what the problem was. Feel free to propose an API that is iterator based!

orsinium commented 5 months ago

I looked at the code and using callbacks is easier from the implementation perspective. There is quite a bit of state that is currently stored in locals of the internal loop but for proper iterators would probably need to be moved into the iterator struct attributes.

Feel free to propose an API that is iterator based!

I'll give it a shot! I'll see if I can add a new read_dir method, to keep backward compatibility.

orsinium commented 5 months ago

I gave it several attempts but my Rust skills aren't good enough to get the job done. There are lots of mutable parts used all over the function (block, block_cache) which makes ownership for iterators tricky, lots of nested loops, and an early escape from inside a loop. In theory, any loop can be an iterator, and I know I can do it in any other language. But in practice, Rust makes it quite hard.

I'll leave the issue open in case someone smarter than me decides to handle it.