ObjectBuilder: add property_if(), property_if_some(), property_from_iter() ...

[fengalin]

... & property_if_not_empty()

This commit adds an ObjectBuilderPropertySetter trait which implements functions to improve usability when setting properties.

property_if() allows setting a property from an Option only if a given predicate evaluates to true. Otherwise, the Object's default value or any previously set value is kept.

property_if_some() allows setting a property from an Option only if the Option is Some. Otherwise, the Object's default value or any previously set value is kept.

For instance, assuming an Args struct which was built from the application's CLI arguments:

let args = Args {
    name: Some("test"),
    anwser: None,
};

... without this change, we need to write something like this:

let mut builder = Object::builder::<SimpleObject>();

if let Some(ref name) = args.name {
    builder = builder.property("name", name)
}

if let Some(ref answer) = args.answer {
    builder = builder.property("answer", &args.answer)
}

let obj = builder.build();

With property_if_some(), we can write:

let obj = Object::builder::<SimpleObject>()
    .property_if_some("name", &args.name)
    .property_if_some("answer", &args.answer)
    .build();

property_from_iter() allows building a collection-like Value property from a collection of items:

let obj = Object::builder::<SimpleObject>()
    .property_from_iter::<ValueArray>("array", ["val0", "val1"])
    .build();

property_if_not_empty() allows building a collection-like Value property from a collection of items but does nothing if the provided collection if empty, thus keeping the default value of the property, if any:

let obj = Object::builder::<SimpleObject>()
    .property_if_not_empty::<ValueArray>("array", ["val0", "val1"])
    .build();

[bilelmoussaoui]

Haven't had the time to review the PR yet but at some point there was a discussion about adding the possibility to set a property only if a condition is truthful. The use case was setting a property only if a certain gstreamer version is available iirc

cc @sdroege, do you remember where was that needed? or maybe it was someone else.

[fengalin]

@bilelmoussaoui: indeed that would be useful. Something like Option::take_if in reverse? I can add that too yes.

[fengalin]

Noteworthy annoyance with these changes is that we need to use glib::prelude::*; (or gst::prelude::*;) in some places were it wasn't before.

[felinira]

I propose renaming property_if_any() to property_if_not_empty() to clarify its purpose and to be more similar to the common is_empty() collection method.

[bilelmoussaoui]

Noteworthy annoyance with these changes is that we need to use glib::prelude::*; (or gst::prelude::*;) in some places were it wasn't before.

That is ok, I already migrated a bunch of files to use prelude instead of including traits manually. Going forward in that direction is good from my point of view

[bilelmoussaoui]

I propose renaming property_if_any() to property_if_not_empty() to clarify its purpose and to be more similar to the common is_empty() collection method.

I would name it property_if_some instead.

[fengalin]

I propose renaming property_if_any() to property_if_not_empty() to clarify its purpose and to be more similar to the common is_empty() collection method

Makes sense.

I would name it property_if_some instead.

Already taken by the Option<_> variant :)

[sdroege]

I propose renaming property_if_any() to property_if_not_empty() to clarify its purpose and to be more similar to the common is_empty() collection method

Makes sense.

Also seems fine to me

[fengalin]

Sorry for the lack of progress on this one. fwiw, I'm planning on working on it during the hackfest.

[fengalin]

Should be ready now

[andy128k]

Another option to keep ergonomic and flexible builder syntax is to use something similar to Kotlin's scope functions.

trait BuilderScopeExt {
    fn apply(self, scope: &mut FnMut(&Self)) -> Self;
}

impl BuilderScopeExt {
    fn apply(mut self, scope: &mut FnMut(&Self)) -> Self {
        (scope)(&mut self);
        self
    }
}

let obj = Object::builder::<SimpleObject>()
     .apply(|builder| {
         if let Some(ref name) = args.name {
              builder.property("name", name);
         }
         if let Some(ref answer) = args.answer {
              builder.property("answer", &args.answer);
         }
     })
     .build();

[fengalin]

@andy128k, thanks for the suggestion! My main intention with this PR was to avoid constructions such as:

    if let Some(ref name) = args.name {
          [...]
    }

Do you want to propose the apply() approach in a separate PR?

[andy128k]

@fengalin There will always be specific inconvenient cases. Now it is Option, later someone may ask for property_if_some_deref, property_if_ok, or property_if_err etc. I'm just saying, instead of polluting builder scope with a variety of corner cases it could be a more universal approach, even if it is a bit more verbose.