rster2002 / ed-journals

Work in progress journal parsing and utilities for Elite Dangerous written in Rust.
https://crates.io/crates/ed-journals
MIT License
3 stars 4 forks source link

Ergonomy: less deeply nested modules #50

Closed plule closed 4 months ago

plule commented 4 months ago

Hey, I've just started a small project where I need to watch screenshots events, and I'm glad ed-journals exists, thanks! Everything is working as expected in my experience, I just want to bring some API suggestions (I can give a shot to implement them, if you think they are appropriate).

I see that there are already some "module shortcuts" that exist, but I think it could be brought into a way flatter structure, leading to a more discoverable API. For instance, the screenshot event content is under: ed_journals::logs::content::log_event_content::screenshot_event::ScreenshotEvent

From the most obvious, to the most contentious, my suggestions would be:

I like this comment on this topic: "users of a module prefer a flat namespace, while implementers prefer hierarchical namespace"

rster2002 commented 4 months ago

Yeah, the logs module was the first module that I created for this project, so it's actually the one that had the least thought put in 😅

I would prefer to keep the event contents like ScreenshotEvent in it's module and not reexport it as there are also a lot of similar looking names that would be 'scoped' under the same module.

I'll probably move things around so that the uses would look something like this:

This would already make uses a lot shorter but still keeps similar looking names scoped to their own module. Does that look good to you?

plule commented 4 months ago

Yes, that's waaay better!

I think I would have expected to have all the events in the same module, like ed_journals::logs::events::ScreenshotEvent, which would be helpful in program that deal with many events, and could be able to use ed_journals::logs::events::* if they wish. But I've not looked into all of the modules to understand if there is a risk of name clash.