Unify builders' patterns

[MichaelScofield]

What type of enhancement is this?

Refactor

What does the enhancement do?

Currently we have a lot of builders with multiple patterns:

• some are using with_field_name to set their values while others are using field_name directly;
• take &mut self and self differently;
• derived from derive-builder crate's builder or not

To make our codes look better, let's unify all these builders' patterns:

• if possible, derived from typed-builder instead of derive-builder. typed-builder looks more promising (see its "Alternatives" section in its readme)
• take self and return self
• use with_field_name

Implementation challenges

No response

[evenyag]

In some circumstances, taking &mut self is more convenient. e.g. pushing some elements into the builder in a loop.

[tisonkun]

@evenyag Can you demostrate the code?

// &mut self

let mut builder = ...;
for ... {
  builder.with_xxx(...);
}

// self
let mut builder = ...;
for ... {
  builder = builder.with_xxx(...);
}

In my illustration, those two flavor in loop seems quite similar.

For add elements -

pub struct Builder {
  ns: Vec<u64>
}

impl Builder {
  fn add_n(&mut self, n: u64) {
    self.ns.push(n);
  }

  fn add_n(mut self, n: u64) -> Self {
    self.ns.push(n);
    self
  }
}

A possible argument is that not always the full ownership is easily to take. This decision happened also when I choose mut key: Vec<u8> from key: &mut Vec<u8> for ChrootKvBackend's methods. But as long as we take control for the world, it should be doable by fostering the codebase.

[evenyag]

The assignment is annoying and looks different from other invocation of setters.

let mut builder = RegionMetadataBuilder::new(self.region_id);
for column in &self.column_metadatas {
    builder = builder.push_column_metadata(column.clone());
}
builder.primary_key(self.primary_key.clone());

We only use the assignment inside the for loop. However, it is just a minor style problem but not a blocker for this issue.

A possible argument is that not always full ownership is easily to take.

Yes, this is possible if we want to reuse the builder or work with a trait object (the MutableVector trait). However, it is possible to unify most builders.