Dependency problem with field changes

Question

The Observer Pattern is used here to notify changes in a Person's data. For example, when age changes, the value of can_vote() can change too, and all observers shall be notified of this change too. A simple if-statement can handle this. But this cannot be maintained when later age affects many other things (e.g. can_drink(), can_quit_school(), etc...), and when many other data members of Person start affecting these and other things too. To handle this explosion in dependencies, I've decided to use std::map<std::string, std::vector<std::string>> dependenciesMap to store a list of things affected by changes in these data members. This is how I implemented this:

#include <iostream>
#include <vector>
#include <map>
#include <tuple>

enum OutputType { Int, Bool, String, NumOutputTypes, FirstType = Int };

template <typename T> struct GetOutputType;
template <> struct GetOutputType<int> : std::integral_constant<OutputType, Int> { };
template <> struct GetOutputType<bool> : std::integral_constant<OutputType, Bool> { };
template <> struct GetOutputType<std::string> : std::integral_constant<OutputType, String> { };

template <typename T>
struct Observer {
    virtual void field_changed (T&, const std::string& field_name, OutputType) = 0;
};

template <typename T>
class Observable {
    std::vector<Observer<T>*> observers;
public:
    template <typename U>
    void notify_field_change (T& source, const std::string& field_name) {
        const OutputType output_type = GetOutputType<U>::value;
        for (Observer<T>* obs : observers)
            obs->field_changed(source, field_name, output_type);
    }
    void subscribe (Observer<T>* f) { observers.push_back(f); }
    void unsubscribe (Observer<T>* f) { observers.erase(remove(observers.begin(), observers.end(), f), observers.end()); }
};

class Person : public Observable<Person> {
    int age, height;
    std::string name, country, zip_code;
    static const std::map<std::string, std::vector<std::string>> dependenciesMap;  // Keeps a list of properties affected by a property.
public:
    Person (const std::string& n, int a, const std::string& c, const std::string& z) : name(n), age(a), country(c), zip_code(z) { }
    static const std::map<std::string, int(Person::*)() const> intMethodMap;
    static const std::map<std::string, bool(Person::*)() const> boolMethodMap;
    static const std::map<std::string, std::string(Person::*)() const> stringMethodMap;
    using TupleMaps = std::tuple<decltype(intMethodMap), decltype(boolMethodMap), decltype(stringMethodMap)>;
    static const TupleMaps methodMaps;
    int get_age() const { return age; }
    void set_age (int a) { set_field(&Person::age, "age", a); }
    int get_height() const { return height; }
    std::string get_name() const { return name; }
    std::string get_country() const { return country; }
    void set_country (const std::string& c) { set_field(&Person::country, "country", c); }
    std::string get_zip_code() const { return zip_code; }
    void set_zip_code (const std::string& z) { set_field(&Person::zip_code, "zip_code", z); }
    bool can_vote() const { return age >= 16 && country == "USA"; }
    bool can_quit_school() const { return age >= 15 && country == "USA" && zip_code == "90210"; }
private:
    template <typename T> void set_field (T Person::*, const std::string& field_name, const T& value);
};
const std::map<std::string, std::vector<std::string>> Person::dependenciesMap = {
    {"age", {"can_vote()", "can_quit_school()"}},  // age affects get_can_vote and can_quit_school.
    {"country", {"can_vote()", "can_quit_school()"}},  // country affects can_vote and can_quit_school.
    {"zip_code", {"can_quit_school()"}}  // zip_code affects can_quit_school
};
const std::map<std::string, int(Person::*)() const> Person::intMethodMap = {
    {"age", &Person::get_age}, {"height", &Person::get_height}
};
const std::map<std::string, bool(Person::*)() const> Person::boolMethodMap = {
    {"can_vote()", &Person::can_vote}, {"can_quit_school()", &Person::can_quit_school}
};
const std::map<std::string, std::string(Person::*)() const> Person::stringMethodMap = {
    {"name", &Person::get_name}, {"country", &Person::get_country}, {"zip_code", &Person::get_zip_code}
};
const Person::TupleMaps Person::methodMaps = std::make_tuple(Person::intMethodMap, Person::boolMethodMap, Person::stringMethodMap);

struct ConsolePersonObserver : Observer<Person> {
    virtual void field_changed (Person& source, const std::string& field_name, OutputType output_type) override { field_changed_helper<FirstType>(source, field_name, output_type); }
private:
    template <OutputType> void field_changed_helper (Person& source, const std::string& field_name, OutputType output_type);
};

template <OutputType N>
void ConsolePersonObserver::field_changed_helper (Person& source, const std::string& field_name, OutputType output_type) {
    if (output_type == N)
        std::cout << source.get_name() << "'s " << field_name << " has changed to " << (source.*std::get<N>(Person::methodMaps).at(field_name))() << ".\n";
    else
        field_changed_helper<static_cast<OutputType>(N+1)>(source, field_name, output_type);
}

template <>
void ConsolePersonObserver::field_changed_helper<NumOutputTypes> (Person&, const std::string&, OutputType) { }  // End of recursion

template <typename T>
void Person::set_field (T Person::*ptr, const std::string& field_name, const T& value) {
    if (value == this->*ptr)
        return;
    std::map<std::string, bool> trackedValues;  // *** All the methods to be invoked through dependenciesMap return bool (if not, then this will be more complicated).
    for (const std::string& str : dependenciesMap.at(field_name))
        trackedValues[str] = (this->*boolMethodMap.at(str))();
    this->*ptr = value;
    notify_field_change<typename std::remove_reference<decltype(this->*ptr)>::type>(*this, field_name);
    for (const std::pair<std::string, bool>& pair : trackedValues) {
        if (pair.second != (this->*boolMethodMap.at(pair.first))())
            notify_field_change<bool>(*this, pair.first);
    }
}

int main() {
    Person p("Tom", 14, "USA", "90210");
    ConsolePersonObserver cpo;
    p.subscribe(&cpo);
    p.set_age(15);
    p.set_age(16);
    p.set_zip_code("999999");
    p.set_country("France");
}

Output:

Tom's age has changed to 15.
Tom's can_quit_school() has changed to 1.
Tom's age has changed to 16.
Tom's can_vote() has changed to 1.
Tom's zip_code has changed to 999999.
Tom's can_quit_school() has changed to 0.
Tom's country has changed to France.
Tom's can_vote() has changed to 0.

Here is my extended code to handle the general case when the output types from the methods obtained by dependenciesMap are different (I removed the template argument of notify_field_change to make this work). For example, country affects can_vote, can_quit_school(), years_left(), and nickname, which return bool, bool, int, and std::string respectively. But I have a feeling there is a cleaner way to handle this than my code below:

#include <iostream>
#include <vector>
#include <map>
#include <tuple>

enum OutputType { Int, Bool, String, NumOutputTypes, FirstOutputType = Int };

template <typename T> struct GetOutputType;
template <> struct GetOutputType<int> : std::integral_constant<OutputType, Int> { };
template <> struct GetOutputType<bool> : std::integral_constant<OutputType, Bool> { };
template <> struct GetOutputType<std::string> : std::integral_constant<OutputType, String> { };

template <typename T>
struct Observer {
    virtual void field_changed (T&, const std::string& field_name, OutputType) = 0;
};

template <typename T>
class Observable {
    std::vector<Observer<T>*> observers;
public:
    void notify_field_change (T& source, OutputType output_type, const std::string& field_name) {
        for (Observer<T>* obs : observers)
            obs->field_changed(source, field_name, output_type);
    }
    void subscribe (Observer<T>* f) { observers.push_back(f); }
    void unsubscribe (Observer<T>* f) { observers.erase(remove(observers.begin(), observers.end(), f), observers.end()); }
};

class Person : public Observable<Person> {
    int age, height;
    std::string name, country, zip_code;
    static const std::map<std::string, std::vector<std::pair<std::string, OutputType>>> dependenciesMap;  // Keeps a list of properties affected by a property.
public:
    Person (const std::string& n, int a, const std::string& c, const std::string& z) : name(n), age(a), country(c), zip_code(z) { }
    static const std::map<std::string, int(Person::*)() const> intMethodMap;
    static const std::map<std::string, bool(Person::*)() const> boolMethodMap;
    static const std::map<std::string, std::string(Person::*)() const> stringMethodMap;
    using TupleMaps = std::tuple<decltype(intMethodMap), decltype(boolMethodMap), decltype(stringMethodMap)>;
    using TupleMaps2 = std::tuple<std::map<std::string, int>, std::map<std::string, bool>, std::map<std::string, std::string>>;
    static const TupleMaps methodMaps;
    int get_age() const { return age; }
    void set_age (int a) { set_field(&Person::age, "age", a); }
    int get_height() const { return height; }
    std::string get_name() const { return name; }
    std::string nickname() const {
        if (country != "France")
            return name;
        return "Francois";
    }
    std::string get_country() const { return country; }
    void set_country (const std::string& c) { set_field(&Person::country, "country", c); }
    std::string get_zip_code() const { return zip_code; }
    void set_zip_code (const std::string& z) { set_field(&Person::zip_code, "zip_code", z); }
    bool can_vote() const { return age >= 16 && country == "USA"; }
    bool can_quit_school() const { return age >= 15 && country == "USA" && zip_code == "90210"; }
    int years_left() const {
        if (country == "France")
            return 10;
        return 80 - age;
    }
private:
    template <typename T> void set_field (T Person::*, const std::string& field_name, const T& value);  // T Person::* is pointer to data member.
    template <OutputType> void set_field_helper (TupleMaps2& trackedValuesAllTypes, const std::pair<std::string, OutputType>&);
    template <OutputType> void notify_other_field_changes (const TupleMaps2&);
};
const std::map<std::string, std::vector<std::pair<std::string, OutputType>>> Person::dependenciesMap = {
    {"age", {{"can_vote()", Bool}, {"can_quit_school()", Bool}, {"years_left()", Int}}},  // age affects can_vote(), can_quit_school(), and years_left().
    {"country", {{"can_vote()", Bool}, {"can_quit_school()", Bool}, {"years_left()", Int}, {"nickname", String}}},  // country affects can_vote, can_quit_school(), years_left(), and nickname.
    {"zip_code", {{"can_quit_school()", Bool}}}  // zip_code affects can_quit_school
 };
const std::map<std::string, int(Person::*)() const> Person::intMethodMap = {
    {"age", &Person::get_age}, {"height", &Person::get_height}, {"years_left()", &Person::years_left}
};
const std::map<std::string, bool(Person::*)() const> Person::boolMethodMap = {
    {"can_vote()", &Person::can_vote}, {"can_quit_school()", &Person::can_quit_school}
};
const std::map<std::string, std::string(Person::*)() const> Person::stringMethodMap = {
    {"name", &Person::get_name}, {"country", &Person::get_country}, {"zip_code", &Person::get_zip_code}, {"nickname", &Person::nickname}
};
const Person::TupleMaps Person::methodMaps = std::make_tuple(Person::intMethodMap, Person::boolMethodMap, Person::stringMethodMap);

struct ConsolePersonObserver : Observer<Person> {
    virtual void field_changed (Person& source, const std::string& field_name, OutputType output_type) override { field_changed_helper<FirstOutputType>(source, field_name, output_type); }
private:
    template <OutputType> void field_changed_helper (Person& source, const std::string& field_name, OutputType output_type);
};

template <OutputType N>
void ConsolePersonObserver::field_changed_helper (Person& source, const std::string& field_name, OutputType output_type) {
    if (output_type == N)
        std::cout << source.get_name() << "'s " << field_name << " has changed to " << (source.*std::get<N>(Person::methodMaps).at(field_name))() << ".\n";
    else
        field_changed_helper<static_cast<OutputType>(N+1)>(source, field_name, output_type);
}

template <>
void ConsolePersonObserver::field_changed_helper<NumOutputTypes> (Person&, const std::string&, OutputType) { }  // End of recursion

template <typename T>
void Person::set_field (T Person::*ptr, const std::string& field_name, const T& value) {
    if (value == this->*ptr)
        return;
    TupleMaps2 trackedValuesAllTypes;
    for (const std::pair<std::string, OutputType>& pair : dependenciesMap.at(field_name))
        set_field_helper<FirstOutputType>(trackedValuesAllTypes, pair);
    this->*ptr = value;
    const OutputType type = GetOutputType<typename std::remove_reference<decltype(this->*ptr)>::type>::value;
    notify_field_change(*this, type, field_name);
    notify_other_field_changes<FirstOutputType>(trackedValuesAllTypes);
}

template <OutputType N>
void Person::set_field_helper (TupleMaps2& trackedValuesAllTypes, const std::pair<std::string, OutputType>& pair) {
    if (pair.second == N)
        std::get<N>(trackedValuesAllTypes)[pair.first] = (this->*std::get<N>(Person::methodMaps).at(pair.first))();
    else
        set_field_helper<static_cast<OutputType>(N+1)>(trackedValuesAllTypes, pair);
}

template <>
void Person::set_field_helper<NumOutputTypes> (TupleMaps2&, const std::pair<std::string, OutputType>&) { }  // End of recursion.

template <OutputType N>
void Person::notify_other_field_changes (const TupleMaps2& trackedValuesAllTypes) {
    for (const auto& pair : std::get<N>(trackedValuesAllTypes)) {
        if (pair.second != (this->*std::get<N>(methodMaps).at(pair.first))())
            notify_field_change(*this, N, pair.first);
    }
    notify_other_field_changes<static_cast<OutputType>(N+1)>(trackedValuesAllTypes);
}

template <>
void Person::notify_other_field_changes<NumOutputTypes> (const TupleMaps2&) { }  // End of recursion.

int main() {
    Person p("Tom", 14, "USA", "90210");
    ConsolePersonObserver cpo;
    p.subscribe(&cpo);
    p.set_age(15);
    p.set_age(16);
    p.set_zip_code("999999");
    p.set_country("France");
}

Output:

Tom's age has changed to 15.
Tom's years_left() has changed to 65.
Tom's can_quit_school() has changed to 1.
Tom's age has changed to 16.
Tom's years_left() has changed to 64.
Tom's can_vote() has changed to 1.
Tom's zip_code has changed to 999999.
Tom's can_quit_school() has changed to 0.
Tom's country has changed to France.
Tom's years_left() has changed to 10.
Tom's can_vote() has changed to 0.
Tom's nickname has changed to Francois.

G. Sliepen · Accepted Answer · 2022-11-19 12:27:30Z

This is a tough one. I can see what you are trying to achieve, and your solution obviously works, but your own spider senses are already tingling that this is not the ideal solution. So why is that?

Move more common functionality into `class Observable`

Your class Observable only handles (un)subscription and notification, but there is more functionality that is common to all observables that you have left up to the derived classes to implement, like set_field(). Ideally, all common things will be moved into the base class.

For example, set_field() could be made a protected member function of Observable, like so:

template <typename T>
class Observable {
    ⋮
protected:
    template<typename Value>
    void set_field(Value T::* ptr, const std::string& field_name, const Value& value);
    ⋮
};

template <typename T> template <typename Value>
void Observable<T>::set_field(Value T::* ptr, const std::string& field_name, const Value& value) {
    T* self = static_cast<T*>(this);

    if (value == self->*ptr)
        return;
    ⋮
}

Note how the base class can get a pointer to the derived class since it knows the type of the latter. That's one of the powers of the CRTP.

You can also move dependenciesMap and the other maps into Observable:

template <typename T>
class Observable {
    static const std::map<std::string, std::vector<std::string>> Person::dependenciesMap;
    ⋮
};

template <>
const std::map<std::string, std::vector<std::string>> Observable<Person>::dependenciesMap = {
    ⋮
};

Whether you want to do that depends; the data can live in either the base class or the derived class, but if you want to avoid it being public, then either the data has to be protected in the base class, or it can be in the derived class but then there must be a friend declaration for the base to be able to access it.

Simplify keeping track of dependencies

Another issue is that you want to have dependencies between fields, but you have to track them somehow. Using dependenciesMap is one way, but I can see this easily getting out of sync, especially because that map is defined in a different place than the setter functions. It's easy to add/remove/modify a setter and then to forget to update the map.

Instead of having a map, consider doing this more programmatically. For example, consider being able to write:

void set_age(int a) {
    set_field(&Person::age, "age", a);
    also_notify(&Person::can_vote, "can_vote()");
    also_notify(&Person::can_quit_school, "can_quit_school()");
}

This would remove the need for the maps.

Avoid unnecessary use of strings

Strings are sometimes used as a cop-out for a more proper solution. They are flexible and can hold anything, but they take up space, and it's easy to make a typo somewhere, but the compiler cannot see that anything is wrong. So if they are not really necessary, try to avoid using them.

Consider your use of strings to record dependencies. Is that really necessary? There is already a way to programmatically refer to members of a class, which you have already been using: pointers to members. Let's go down the rabbit hole!

Instead of std::string field_name, we want to use a pointer-to-member function. However, there is a problem: not all pointers-to-member have the same type, and you cannot type-erase a pointer-to-member by casting it to void*. You could use something else. Consider using a std::variant. While that was introduced in C++17, you could make your own that is compatible with C++11, or use boost::variant. You can then write:

using PointerToMember = std::variant<int Person::*, std::string Person::*, ...>;

But while this would work up to a point, you can't use them as a key in a std::map, since these types are not comparable. Instead of trying to type erase it inside Person or Observable, let's just see how far we get pushing the problem to other layers. For now, we can just keep using templates to handle arbitrary pointer-to-member types.

In set_age(), let's remove the strings, and we just write:

void set_age(int a) {
    set_field(&Person::age, a);
    also_notify(&Person::can_vote);
    also_notify(&Person::can_quit_school);
}

Since we call also_notify() explicitly, set_field() would not need to track dependencies itself, it just needs to update the value if necessary, and then notify any subscriber of that value:

template <typename T> template <typename Value>
void Observable<T>::set_field(Value T::* ptr, const Value& value) {
    T* self = static_cast<T*>(this);
    if (self->value == *ptr)
        return;
    self->*ptr = value;
    also_notify(ptr);
}

And the function also_notify(), perhaps a bit misnamed now, should take care of actually notifying observers:

template <typename T> template <typename Value>
void Observable<T>::also_notify(Value T::* ptr) {
    for (auto observer: observers)
        observer->field_changed(static_cast<T*>(this), ptr);
}

Here we run in to trouble. Because Observer::field_changed() is a virtual method, it cannot be a template. So this is where we should use type-erasure to make sure we don't need templates. So:

template <typename T>
struct Observer {
    virtual void field_changed(T::PointerToMember ptr) = 0;
};

Note that it's the struct that is templated, not its member function. Of course this places a burden on the observer to actually deal with the variant and compare the pointer-to-members. For example:

struct PersonObserver: Observer<Person> {
    override void field_changed(Person::PointerToMember ptr);
};

void PersonObserver(Person::PointerToMember ptrv) {
    if (std::holds_alternative<int Person::*>(ptrv)) {
        int Person::* ptr = std::get<int Person::*>(ptr);
        if (ptr == &Persion::age) {
            std::cout << "Age changed to " << *ptr << "\n";
        }
        ⋮
    }
    ⋮
}

This can be done more properly with visitors.

Consider making each field a separate observable

A lot of the complexity from your class comes from the fact that you want to be able to make the whole class an observable. This complexity can be greatly reduced by making each field itself an observable. This way, there is only one type to deal with for each observable, and an observer can just observe a single field, which is probably what you want anyway. Consider being able to write:

class Person {
public:
    Observable<int> age;
    Observable<int> height;
    Observable<std::string> name;
    ⋮        
};
⋮        
int main() {
    Person p(…);
    p.age.subscribe([](Person *p) {
        std::cout << "The age of " << p->name << " has changed to " << p->age << '\n';
    });
    p.age = 15;
}

This can be done, and you could add a way to register getter and setter functions at construction time that will allow you to get derived values and notify dependencies. The drawback is that there might be a large overhead per observable: pointers to the getter and setter function, as well as a std::vector containing pointers to observers.

Edward · Accepted Answer · 2022-11-19 12:07:35Z

It's an interesting question, and the code is a little complex but not bad. Here are some things that may help you improve it.

Initialize all members

The constructor for Person initializes every data member except height. Better would be to initialize all members, passing each in via constructor arguments.

Separate underlying data from mechanism

Somewhere in there is a Person trying to get out. :) Seriously, the Person class has a few items that actually relate to a person, but most of the code is the observer mechanism. It would be far nicer to use if all or most of the observer mechanism were hidden in the Observable class.

Collect related things in a class

Looking at an individual field, age, there are a number of things related to it that are related such as the string label "age", and the various derived fields. I would suggest refactoring the code to have observable fields and then compose objects from these. Fields could then observe other fields and external objects could then observe individual data items rather than exclusively whole objects.

Consider using `std::variant`

If you can move from C++11 to at least C++17, you could use std::variant which is likely to make things simpler than having to keep an OutputType enum and all of the associated mechanisms.

Reconsider using string lookup

I understand the design choice of using std::strings as the key to your various maps, but it seems fragile and unwieldy. A more direct mechanism such as the ones suggested below could eliminate the use of strings for this purpose.

Consider using a signal and slot design

Both Qt and D-Bus use a signal and slot design whereby objects declare signals that can then be connected to other objects containing slots with matching parameters.

Consider using a "property" design

The property design is one that is most easily constructed atop a signal-and-slot design. Each property has set and get methods (unless it's designated read only) and an "on changed" signal. This greatly streamlines and simplifies the means of defining things like your current can_quit_school(). It would be defined as a read-only property that emits an "on changed" signal if it receives changes from the underlying age, country or zip_code properties and if one or more of those changes alters the calculated value. The really interesting thing about this is that derived types can easily add their own synthetic properties.

Stack Exchange Network

Dependency problem with field changes

2 Answers 2

Move more common functionality into `class Observable`

Simplify keeping track of dependencies

Avoid unnecessary use of strings

Consider making each field a separate observable

Initialize all members

Separate underlying data from mechanism

Collect related things in a class

Consider using `std::variant`

Reconsider using string lookup

Consider using a signal and slot design

Consider using a "property" design

Not the answer you're looking for? Browse other questions tagged
c++
c++11
observer-pattern
or ask your own question.

Hot Network Questions

Dependency problem with field changes

2 Answers 2

Move more common functionality into class Observable

Simplify keeping track of dependencies

Avoid unnecessary use of strings

Consider making each field a separate observable

Initialize all members

Separate underlying data from mechanism

Collect related things in a class

Consider using std::variant

Reconsider using string lookup

Consider using a signal and slot design

Consider using a "property" design

Not the answer you're looking for? Browse other questions tagged c++c++11observer-pattern or ask your own question.

Related

Hot Network Questions

Move more common functionality into `class Observable`

Consider using `std::variant`

Not the answer you're looking for? Browse other questions tagged
c++
c++11
observer-pattern
or ask your own question.