altugbakan / rs-tftpd

TFTP Server Daemon implemented in Rust
https://crates.io/crates/tftpd
MIT License
51 stars 14 forks source link

feat(config): Implement std::default::Default #19

Closed BKDaugherty closed 3 months ago

BKDaugherty commented 3 months ago

Hey! I'm hoping to use this library for a project of mine, and wanted to instantiate a Config using the Default trait. Thought I'd put up a PR for it! Lmk what you think!

stappersg commented 3 months ago

(Here not the project lead, some random Joe) I see only lines moved, I don't see any improvement. Most likely due me not understanding the "I'd like to use Config::default in my own code when constructing a Config. Let's implement the Default trait so that others can use it without wrapping our struct due to ownership rules." commit message.

BKDaugherty commented 3 months ago

Most likely due me not understanding the "I'd like to use Config::default in my own code when constructing a Config. Let's implement the Default trait so that others can use it without wrapping our struct due to ownership rules." commit message.

Ah. @stappersg, no worries! Happy to explain :)

It's fairly common in Rust for types to implement certain Traits, (or interfaces in other languages), for standard behaviors. One of those behaviors is constructing a default representation of a type. Check out the docs on the trait here: https://doc.rust-lang.org/std/default/trait.Default.html

I typically do this when I'd like to create a config with specific values, but I don't really care about the other values.

Let's take for example someone wanting to run their TFTP server on the IPV6 localhost (::1), but they want to just spin up the rest of the config to the default values. (Say for example, they don't know that the port should be 69, or don't want to duplicate that constant in their own code.) They can use the following syntax

let config = Config {
  ip_address: IpAddr::V6(Ipv6Addr::LOCALHOST),
  ..Config::default()
};

And rely on the default implementation to do the rest.

It's possible to do something similar to the above with the existing Config::new method, but a little awkward, and unexpected compared to the rest of the ecosystem, which relies on the standard library's trait.

    let config = Config {
        ip_address: IpAddr::V6(Ipv6Addr::LOCALHOST),
    ..Config::new([].into_iter()).expect("Config building with no args should be fine, and my application depends on that")
    };
altugbakan commented 3 months ago

Looks great initially, I’ll take a closer look when I get home. Thank you for your addition!