First project, API wrapper

code review
1

Hello!
As my first Rust project I've implemented simple wrapper for delivery service API. I would appreciate your feedback!
Github

2

Some quick observations:

  • You have redundant implementations: TryFrom<String> for ENumber and TryFrom<&str> for ENumber. Instead, you should implement FromStr.

  • This implementation is not useful:

impl From<ENumber> for String {
    fn from(en: ENumber) -> Self {
        en.0
    }
}

It's simpler to use en.0 than en.into().

  • Avoid "stringified" types. Do as you did with ENumber for these other properties:
pub struct Counterparty {
    description: String,
    first_name: String,
    middle_name: String,
    last_name: String,
    counterparty: Option<String>,
    ownership_form: Option<String>,
    ownership_form_ref: Option<String>,
    ownership_form_description: Option<String>,
    EDRPOU: Option<String>,
    ...
    city: Option<String>,
}

  • Also try to not use types such as Uuid directly. This is very error prone since later on you could pass the id from the wrong entity. Use the NewType pattern here as well.

  • You can leverage the FromStr implementations for deserialization with DeserializeFromStr from serde_with.

1 Like
3

Thank you so much for your feedback on my project! Especially about not using Uuid directly, it is indeed very error prone :disappointed:
I'll fix it according to your suggestions!
Thanks again!

4

That's not really true. A From/Into impl can still be useful in a generic context.

5

Can you further explain? I fail to think of a scenario where passing the internal string would fail.

6

Maybe someone has a

impl TheirStruct<S>
where
    S: Into<String>
7

Interesting :thinking:. Yeah, that could be one scenario definitely.

8

Checked your project again and found one last thing to comment on (Edited my original post to add it). In general I think your project is clean and well structured :smiley: .

2 Likes
9

I gave a quick look, and importing/mod statements seem incorrect.

First, binary is like a crate on its own, and shouldn't have mod statements, unless they're inside the same file (e.g. main.rs). It can import modules from the crate via use my_crate::... statement, just like modules/struct from 3rd-party crates.

mod statement says that current lib.rs/mod.rs also provides a submodule from another file. (or it's mod { <mod content> } clause).

Basic structure of crate is:

my_crate/
    Cargo.toml
    src/
        lib.rs:
            [pub] mod another_module
            [pub] mod third_module
            [pub] mod subdir
        another_module.rs: ...
        third_module.rs: ...
        subdir/
            mod.rs:
                [pub] mod fourth_module
            fourth_module.rs
        
        main.rs: // lives on its own
            use my_crate::{another_module, subdir::fourth_module};

Also, you can put tests closer to the tested modules, e.g. in lib.rs and submodules, and in main.rs as well (to test the CLI api):

// some module rs
pub struct MyStruct { ... }
...
#[cfg(test)]
mod my_module_tests {
     use super::*;
     #[test]
     fn it_works() {
         ...
     }
}
10

That is not correct. Why couldn't a binary have modules too? How else would you organize the code? Not everything a binary application needs should be bundled in a library. It would be perfectly fine to put e.g. CLI argument parsing or configuration in its own module.

2 Likes