Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider connection_owner idea from PG1003/observer #131

Open
YarikTH opened this issue Sep 4, 2023 · 0 comments
Open

Consider connection_owner idea from PG1003/observer #131

YarikTH opened this issue Sep 4, 2023 · 0 comments

Comments

@YarikTH
Copy link
Owner

YarikTH commented Sep 4, 2023

Description

See
https://github.com/PG1003/observer#manage-multiple-connections-using-a-pgconnection_owner

After redesigning of observer ownership scheme, we get some inconvenient state where do make N observations and keep them alive we need N uniquely named observers.

For example (run)

#include <ureact/signal.hpp>
#include <ureact/adaptor/lift.hpp>
#include <ureact/adaptor/observe.hpp>
#include <iostream>

using namespace ureact;
using namespace ureact::default_context;

class Shape  // <1>
{
public:
    UREACT_USE_MEMBER_SIGNALS(Shape);

    Shape() = default;

    member_var_signal<int> width  = make_var( 1 );
    member_var_signal<int> height = make_var( 1 );
    member_signal<int>     size   = width * height;
};

int main()
{
    Shape myShape;

    // <3>
    // Callback on change
    auto obsSize = observe( myShape.size,
        []( int newSize )   { std::cout << "size -> "   << newSize   << "\n"; } );

    // Those would work, too
    auto obsWidth = observe( myShape.width,
        []( int newWidth )  { std::cout << "width -> "  << newWidth  << "\n"; } );
    auto obsHeight = observe( myShape.height,
        []( int newHeight ) { std::cout << "height -> " << newHeight << "\n"; } );

    // <2>
    // Set dimensions
    myShape.width( 20 );
    myShape.height( 20 );

    // Get size
    const auto curSize = myShape.size();
    std::cout << "curSize: " << curSize << "\n";
}

We have to declare obsSize, obsWidth and obsHeight separately. To have our observations. It wasn't the case before, when observer nodes were owned by observable signals too.

The same is actual for classes with observers. It's cumbersome to make N named observers if we don't want to detach some of them manually, but we just need to bind their life time to the owner class.

So, observer_owner or observer_set to the rescue:

int main()
{
    observer_owner connect;
    Shape myShape;

    // <3>
    // Callback on change
    connect = observe( myShape.size,
        []( int newSize )   { std::cout << "size -> "   << newSize   << "\n"; } );

    // Those would work, too
    connect = observe( myShape.width,
        []( int newWidth )  { std::cout << "width -> "  << newWidth  << "\n"; } );
    connect = observe( myShape.height,
        []( int newHeight ) { std::cout << "height -> " << newHeight << "\n"; } );
}

I'm not sure about name and semantics, but it is the general idea. Also, need to think about inheritance from observer_owner.


Does it makes sense or better to rely on observer_owner field in class? I fear, that if observer_owner would be the base class, then all observers pointing to the class members will be alive between destruction of the class, and destruction of base observer_owner class. It might be a problem if the class' destructor or destructors of the fields will trigger ureact's change propagation that triggers our observers bound to the dying class.

So, it seems that aggregation works better in this case.


It's temping to declare some global observer_owner inline constexpr instance so each cout observer or similar observer with unlimited lifetime can be easily bound to it without creation of several specific ones. But I don't know how to handle multighteading. Attaching and detaching can be performed from any thread in general case, so it should be handled somehow.

Does thread_local constants and variables desctroyed when thread is closed? If so, then maybe such global holder should also be thread_local (but it is hard to have detach_all method this way).


To not bloating observer_owner with infinite amount of already expired observers, we need some kind of clean up mechanism. I'm not sure should it be some kind of std::remove_if on any observer attachment or it should be node based (node in each context which depend on controlled observer nodes and auto detached from expired ones if it is triggered).

The life becomes easier if EOS optimization feature would be implemented. Then observes on constant signal trees and completed event streams would be auto detached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant