colin-kiegel / rust-derive-builder

derive builder implementation for rust structs
https://colin-kiegel.github.io/rust-derive-builder/
Apache License 2.0
1.28k stars 82 forks source link

Allow opting out of impl Default #228

Closed TedDriggs closed 2 years ago

TedDriggs commented 2 years ago

This adds #[builder(custom_constructor)] as a way to suppress the generation of impl Default for FooBuilder. The name is chosen to avoid confusion with #[builder(default)], as that is about the usage of the struct-level default to fill in missing fields.

One positive emergent property from this is that it's easy to put the new method onto the target type, rather than the builder: ApiClient::new(...) rather than ApiClientBuilder::new(...). This seems advantageous for managing the set of imports needed to use ApiClient.

One unexpected aspect is the need to add #[builder(setter(custom))] to each set-up-front field. While it does make the struct more verbose, I think it illustrates how many nuances there are to this behavior, and it ends up supporting the idea that this is something people should create for themselves on top of the tools derive_builder provides, rather than attempting to generate constructors entirely through attributes.

Alternative API

This would also work: #[builder(constructor(private, name = "create_empty))]. That would remove the need to remember or dig out the create_empty method name each time, at the expensive of additional verbosity in the attribute declaration.

Fixes #227

TedDriggs commented 2 years ago

@CarlKCarlK please take a look at this PR in relation to this

With the changes here, that code would now read as follows:

#[derive(Builder)]
#[builder(custom_constructor, build_fn(private, name = "try_build"))]
pub struct Bed {
    #[builder(setter(custom))]
    path: PathBuf,
    // ... other fields elided for brevity
}

impl Bed {
    pub fn builder<P: AsRef<Path>>(path: P) -> BedBuilder {
        BedBuilder::new(path)
    }
}

impl BedBuilder {
    pub fn new<P: AsRef<Path>>(path: P) -> Self {
        Self {
            path: Some(PathBuf::from(path.as_ref())),
            ..Self::create_empty()
        }
    }

    pub fn build(&self) -> Result<Bed, BedErrorPlus> {
        let bed = self.try_build()?;

        let mut buf_reader = BufReader::new(File::open(&bed.path)?);
        let mut bytes_vector: Vec<u8> = vec![0; CB_HEADER_USIZE];
        buf_reader.read_exact(&mut bytes_vector)?;
        if (BED_FILE_MAGIC1 != bytes_vector[0]) || (BED_FILE_MAGIC2 != bytes_vector[1]) {
            return Err(BedError::IllFormed(PathBuf::from(&bed.path).display().to_string()).into());
        }

        Ok(bed)
    }
}

You'd also be able to remove the UninitializedFieldError variant from BedErrorPlus, as it would no longer be possible for BedBuilder to produce one.

Some questions:

  1. Is this something you'd adopt? If not, can you explain your concerns so that I don't merge something bad here?
  2. What documentation would you have found most helpful for this scenario?