almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
108 stars 47 forks source link

Removing the `with_` prefix from builder methods #113

Closed rfuest closed 4 months ago

rfuest commented 4 months ago

Should we remove the with_ prefix from the builder methods? The API guidelines state that the with_ prefix should be used for constructors (https://rust-lang.github.io/api-guidelines/naming.html#casing-conforms-to-rfc-430-c-case) and builders usually don't use and prefix for the methods (https://rust-lang.github.io/api-guidelines/type-safety.html?highlight=builder#builders-enable-construction-of-complex-values-c-builder).

The naming is confusing in the latest release where Builder::with_model is a constructor, while other methods, like Builder::with_orientation, are used to set a parameters. In the master branch with_model was renamed into new, which makes it harder to mix up the two types of methods, but I would still prefer to remove the prefix.

almindor commented 4 months ago

Hmm so you mean it'd look like Builder::new(<model>).orientation(Orientation::Portrait)... ? I'm a bit on the fence. Something like Builder::new(...).orientation(Orientation::Portrait) seems a bit odd to me

rfuest commented 4 months ago

Yes, that's what I meant. It is the recommended way of naming builder methods and should be familiar to many users, because it is used in many places like https://doc.rust-lang.org/std/process/struct.Command.html, https://docs.rs/clap/latest/clap/struct.Arg.html or https://docs.rs/embedded-graphics/latest/embedded_graphics/text/struct.TextStyleBuilder.html.