PropertyDescriptionBuilder: check that format is specified for types that require them?

[nagisa]

Currently the PropertyDescriptionBuilder does not appear to have any failure conditions. However there is at least one condition that a valid property description must satisfy -- if the type is of enum or color, then the format must be specified describing valid variants (for enum) or color formats (for color.)

It looks like today there is nothing preventing an invalid property from being constructed. I can think of a couple of possible avenues here:

1. Move the field describing the valid values to HomieDataType::Enum and HomieDataType::Color. This makes the format impossible to miss specifying, but at the same time it leads to a weird split between types that have an optional format. It also doesn't directly follow the shape of the description JSON anymore;
2. Make PropertyDescriptionBuilder::build fallible (return a Result.) It still makes it possible to construct invalid PropertyDescriptions by building the PropertyDescription structure directly due to its public fields, but perhaps that might even be desired -- if only to handle other device implementations which might have not specified their enum/color formats for whatever reason.

[schaze]

yes you are right. Tbh I did not put a lot of work into the builders yet....

I am thinking more towards a typestate builder pattern like here: https://blog.ediri.io/design-patterns-in-rust-upgrading-the-builder-pattern-using-the-typestate-pattern
Do you think this would work?

[nagisa]

Typestate is nice theoretically, but it can at times be difficult to understand what the compiler is complaining about when you get the requirements imposed by the typestate wrong. In my mind nowadays it is perhaps better to work towards making the entire builder const {} if at all possible and panic in the build() method. Still fails at compile time, but gives great diagnostics when things go wrong. At the same time limiting the code to const makes it pretty awkward to write and many tasks are not possible at all quite yet.