mre / prettyprint

Beautifully formatted output for your terminal 🌈
Apache License 2.0
200 stars 10 forks source link

Refactor builder pattern #12

Closed DrSensor closed 5 years ago

DrSensor commented 5 years ago

Hi, notice that the builder pattern is a bit daunting when I want to print 2 different things.

For example, I need to create 2 instances for printing 2 things (partial result + error messages): ```rust let printer = PrettyPrinter::default() .header(false).grid(false) .paging_mode(PagingMode::Never) .language("erlang").build()?; let eprinter = PrettyPrinter::default() .header(false).grid(false) .paging_mode(PagingMode::Error) .language("rust").build()?; if !error { printer.string(partial_result); } else { eprinter.string_with_header(error_messages, filename); } ``` I'm not sure about πŸ‘† but it probably can cause memory spike πŸ€” (because there is 2 instances)

I'm thinking to refactor it so that it can switch the configuration without doing instantiation.

Something akin to this: ```rust let printer = PrettyPrinter::default() .header(false).grid(false) .paging_mode(PagingMode::Never) .language("erlang").build()?; if !error { printer.string(partial_result); } else { printer.configure().paging_mode(PagingMode::Error).language("rust"); // this will change the configuration printer.string_with_header(error_messages, filename); } // πŸ‘ˆ maybe when out of scope, printer configuration will revert back // (which mean `PrettyPrint` will implement trait `Drop`) ```

However, is it possible to do it without introducing breaking changes?

mre commented 5 years ago

However, is it possible to do it without introducing breaking changes?

It should be possible I think but a bit tricky to implement. build() returns an instance of PrettyPrint and calling configure() on it would mean we need access to the builder again. So either the builder is a member of PrettyPrint which would be tricky to test or we create a new instance of the builder which could work. It would look a bit like this, I guess:

fn configure(self) -> PrettyPrinter {
  PrettyPrinter::from(self)
}

You can give it a shot. Maybe as a preliminary step, we could implement From<PrettyPrint> for PrettyPrinter and see how that goes?