firestore-db-and-auth-rs
firestore-db-and-auth-rs copied to clipboard
Better naming convention for `Result` type alias in `errors` module
Is your feature request related to a problem? Please describe.
Would it be possible to have a better type alias name for the firestore_db_and_auth::errors::Result type? As a relative newcomer to Rust this was extremely unclear how to use this. Currently the module errors declares a type alias that is intended to capture a variety of different errors (see original source code)
// Current implementation
pub type Result<T> = std::result::Result<T, FirebaseError>;
It was very unclear to me that where I would call operations like document::read() that I had to use the module's Result and not std::result. This is not clearly laid out in the documentation anywhere. Based on the example below, I'm not sure that the intention of this change lead to better ergonomics
// Incorrect
let result: Result<MyFirestoreUser, firestore_db_and_auth::errors::FirebaseError> = documents::read(&session, "user", "123456");
// Correct
let result: firestore_db_and_auth::errors::Result<MyFirestoreUser> = documents::read(&session, "user", "123456");
Describe the solution you'd like
Perhaps just declare this alias as something more specific to this crate FirestoreResult?
I think this keeps the ergonomic efficiency and also makes it clear you're getting a different Rust type as a result of the operation.
let result: FirestoreResult<MyFirestoreUser> = documents::read(&session, "user", "123456");
Describe alternatives you've considered
Is there a way to import this type into lib/main.rs so that I don't have to properly scope every call to firestore_db_and_auth:errors::Result
Additional context
Running on v0.6.0
Hi thanks for the feedback.
When I had designed the API this way of having an own Result type within an error module was kind of state of the art or let's say the most Rust idiomatic way. Error handling has changed since then in the community.
So I will give it a second thought here as well. Thanks again for the suggestions, maybe I'm going with these.
If you're open to the suggestion I'm happy to take a pass at making the change. I think the more controversial decision would be about the new type name - FirestoreResult? Definitely understand that this was idiomatic Rust (and may still be) but I do think this would make it more intuitive and would make the code more easily understood. Let me know!
I will add a FirestoreResult to the libraries directly/non-namespaced exported types as an alias for errors::Result.
(also recommended by https://doc.rust-lang.org/rust-by-example/error/result/result_alias.html)
It is still idiomatic Rust to have a Result type somewhere (see https://doc.rust-lang.org/rust-by-example/error/multiple_error_types/define_error_type.html#defining-an-error-type), so I will not remove the existing one.