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

Make Directory::iterate_dir version returning an Iterator #125

Open
orsinium opened this issue Apr 2, 2024 · 3 comments
Open

Make Directory::iterate_dir version returning an Iterator #125

orsinium opened this issue Apr 2, 2024 · 3 comments

Comments

@orsinium
Copy link
Contributor

orsinium commented Apr 2, 2024

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
Copy link
Member

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
Copy link
Contributor Author

orsinium commented Apr 3, 2024

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
Copy link
Contributor Author

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.

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

2 participants