GregoryConrad / rearch-rs

Re-imagined approach to application design and architecture
https://crates.io/crates/rearch
MIT License
84 stars 4 forks source link

feat: add `overridable_capsule` side effect #64

Closed GregoryConrad closed 3 months ago

GregoryConrad commented 3 months ago

See #62

GregoryConrad commented 3 months ago

CC @busslina, I'll publish a new rearch-effects with this addition in the next couple hours

busslina commented 3 months ago

I just migrated 90% of my code to use this abstraction (which is great) but I have an issue. I have a lib level function that overrides two capsules and I need to use DynCapsuleHolder. What do you think about making it public?

pub fn override_capsules(
    app_name_capsule: DynCapsuleHolder<String>,
    app_config_file_capsule: DynCapsuleHolder<String>,
) {
    // App name
    CAPSULE_CONTAINER.read(app_name_overridable_capsule).1(app_name_capsule);

    // App config file
    CAPSULE_CONTAINER
        .read(app_config_file_overridable_capsule)
        .1(app_config_file_capsule);
}
GregoryConrad commented 3 months ago

@busslina I’d recommend using the new effect I added to rearch-effects (overridable_capsule) instead of the code snippet I gave before; it’s a little more robust.

But regardless, you can just pass in the capsules themselves (using generics or the impl Trait syntax). The new OverridableCapsule and the DynCapsuleHolder I gave in the code snippet earlier can be constructed from regular Capsules.

busslina commented 3 months ago

I’d recommend using the new effect I added to rearch-effects (overridable_capsule) instead of the code snippet I gave before; it’s a little more robust.

I'm using it:

// -------------------------------------------------------------------------
// App name
// -------------------------------------------------------------------------
pub fn app_name_capsule(mut handle: CapsuleHandle) -> String {
    let capsule = handle.get.as_ref(app_name_overridable_capsule).clone();
    handle.get.as_ref(capsule).to_owned()
}

pub fn app_name_overridable_capsule(handle: CapsuleHandle) -> OverridableCapsule<String> {
    handle
        .register
        .register(overridable_capsule(app_name_capsule_default))
}

fn app_name_capsule_default(_: CapsuleHandle) -> String {
    panic!("App name capsule must be override");
}
// -------------------------------------------------------------------------

Just that I have a lib helper function that handles the override initialization. The function content is outdated. I was just focusing on the function signature that uses DynCapsuleHolder. Anyways, I will try to refactor it to use impl Capsule, but seems maybe util to be able to use DynCapsuleHolder.

busslina commented 3 months ago

Okay, I think I'm wrong, and I can't use DynCapsuleHolder anymore with this new abstraction.

busslina commented 3 months ago

Solved:

pub fn override_capsules(
    app_name_capsule: impl Capsule<Data = String> + CData,
    app_config_file_capsule: impl Capsule<Data = String> + CData,
) {
    // App name
    CAPSULE_CONTAINER
        .read(app_name_overridable_capsule)
        .set(app_name_capsule);

    // App config file
    CAPSULE_CONTAINER
        .read(app_name_overridable_capsule)
        .set(app_config_file_capsule);
}

Good job! :)

GregoryConrad commented 3 months ago

Solved:

pub fn override_capsules(
    app_name_capsule: impl Capsule<Data = String> + CData,
    app_config_file_capsule: impl Capsule<Data = String> + CData,
) {
    // App name
    CAPSULE_CONTAINER
        .read(app_name_overridable_capsule)
        .set(app_name_capsule);

    // App config file
    CAPSULE_CONTAINER
        .read(app_name_overridable_capsule)
        .set(app_config_file_capsule);
}

Good job! :)

Looks about right! Only recommendation is to change CData to Sync, since that's the only extra bound you need.