feature: builder pattern for application

[WhyNotHugo]

Creating an Application instance is pretty noisy:

    let immediate_alert_service: Uuid = Uuid::from_u16(IMMEDIATE_ALERT_SERVICE);
    let alert_level: Uuid = Uuid::from_u16(ALERT_LEVEL);

    let app = Application {
        services: vec![Service {
            uuid: immediate_alert_service,
            primary: true,
            characteristics: vec![Characteristic {
                uuid: alert_level,
                write: Some(CharacteristicWrite {
                    write_without_response: true,
                    method: CharacteristicWriteMethod::Fun(Box::new(move |new_value, _req| {
                        async move {
                            println!("Got an alert with value: {:?}", new_value);
                            Ok(())
                        }
                        .boxed()
                    })),
                    ..Default::default()
                }),
                ..Default::default()
            }],
            ..Default::default()
        }],
        ..Default::default()
    };

All these Default::default add lot of noise to reading the code (I'm pretty sure the compiler actually optimised away all the intermediate variables being created here).

After writing a small application using this type, I have a strong impression that a much better API could be provided by using a builder pattern:

    let immediate_alert_service: Uuid = Uuid::from_u16(IMMEDIATE_ALERT_SERVICE);
    let alert_level: Uuid = Uuid::from_u16(ALERT_LEVEL);

    let app = Application::buider()
        .add_service(
            Service::builder(immediate_alert_service).with_primary_characteristic(
                Characteristic::builder(alert_level).write_without_response(
                    true,
                    CharacteristicWriteMethod::Fun(Box::new(move |new_value, req| {
                        async move {
                            todo!("{:?} {:?}", new_value, req);
                        }
                        .boxed()
                    })),
                ),
            ),
        )
        .build();

Note how this approach also makes some fields mandatory (e.g.: the uuid to Application must be provided). Aside from with_primary_characteristic, the ServiceBuilder also has a with_characteristic for non-primary ones.

A longer (fictitious) example:

    let app = Application::builder()
        .add_service(
            Service::builder(immediate_alert_service)
                .with_primary_characteristic(
                    Characteristic::builder(alert_level).write_without_response(
                        true,
                        CharacteristicWriteMethod::Fun(Box::new(move |new_value, req| {
                            async move {
                                todo!("{:?} {:?}", new_value, req);
                            }
                            .boxed()
                        })),
                    ),
                )
                .with_characteristic(
                    Characteristic::builder(another_uuid).write_without_response(
                        true,
                        CharacteristicWriteMethod::Fun(Box::new(move |new_value, req| {
                            async move {
                                todo!("{:?} {:?}", new_value, req);
                            }
                            .boxed()
                        })),
                    ),
                ),
        )
        .add_service(
            Service::builder(another_service).with_primary_characteristic(
                Characteristic::builder(some_other_uuid).write_without_response(
                    true,
                    CharacteristicWriteMethod::Fun(Box::new(move |new_value, req| {
                        async move {
                            todo!("{:?} {:?}", new_value, req);
                        }
                        .boxed()
                    })),
                ),
            ),
        )
        .build();

Is there any interest in having such builders? I'd start out with the inner ones (e.g.: Characteristic::builder) and progressively add the higher one.

[surban]

This would be a good extension to the API.

It would also make sense to provide a wrapper function around CharacteristicWriteMethod::Fun and similar types that perform the boxing of the specified closure, i.e. Box::new(move |...| { async move { ... }.boxed() }).

I'd start out with the inner ones (e.g.: Characteristic::builder) and progressively add the higher one.

Yes, we can then discuss the proposed builder pattern in more detail.

[WhyNotHugo]

It would also make sense to provide a wrapper function around CharacteristicWriteMethod::Fun and similar types that perform the boxing of the specified closure, i.e. Box::new(move |...| { async move { ... }.boxed() }).

Yeah, that's also kind ugly and repetitive. Ideally, the following:

CharacteristicWriteMethod::Fun(Box::new(move |new_value, req| {
    async move {
        todo!("{:?} {:?}", new_value, req);
    }
    .boxed()
})),

Could be written as:

CharacteristicWriteMethod::from(async move {
    todo!("{:?} {:?}", new_value, req);
}),

However, async closures are unstable: https://github.com/rust-lang/rust/issues/62290.

I used bluer/examples/gatt_server_cb.rs for some experimenting around simplifying this bit anyway. I ended up hitting lifetime issues that come up due to the async block moving variables into its scope.

I'm not sure that there's anything actionable on simplifying CharacteristicWriteMethod definition.

[surban]

You don't need async closures for that. You can create a wrapper function like this:

use futures::future::FutureExt;
use std::future::Future;
use std::io::Result;
use std::pin::Pin; // 0.3.30

pub type CharacteristicWriteFun =
    Box<dyn Fn(Vec<u8>, String) -> Pin<Box<dyn Future<Output = Result<()>> + Send>> + Send + Sync>;

pub enum CharacteristicWriteMethod {
    Fun(CharacteristicWriteFun),
    Io,
}

impl CharacteristicWriteMethod {
    pub fn fun<Fut>(f: impl Fn(Vec<u8>, String) -> Fut + Send + Sync + 'static) -> Self
    where
        Fut: Future<Output = Result<()>> + Send + 'static,
    {
        Self::Fun(Box::new(move |a, b| f(a, b).boxed()))
    }
}

fn main() {
    CharacteristicWriteMethod::fun(|a, b| async move { Ok(()) });
}

See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e1d0f1b90e94971cade64cc536713070