Kiril
Kiril

Reputation: 40385

Multiple compiler errors while trying to implement the observer pattern

I posted a question earlier on how to cast a member function to a typedef function pointer ( How to use a typedef function pointer to register a callback ) and it prompted me to consider a slightly modified solution by implementing the "classic" observer pattern.

I want to allow class A to notify observers who get a callback via the function pointer and observers who get a callback via a member function, but I can't figure out what's const-correct way to do this. I've looked at other examples online and now I'm just confused with where I'm going wrong.

Here is an sscce compliant example that I pasted in ideone.com (with the compiler errors below it):

#include <set>

typedef int (*CallbackEvent)(const char*, const char*, int);

// Observer interface
class IObserver
{
public: 
    virtual ~IObserver(){}
    virtual int CallMeBack(const char*, const char*, int);
};

class A
{
private:
    std::set<IObserver> observers;
public:
    A(){}
    ~A(){}
    void RegisterObserver(const IObserver& observer)
    {
        observers.insert(observer);
    }

    inline void UnregisterObserver(const IObserver& observer)
    {
        observers.erase(observer);
    }

    void NotifyAll()
    {
        for(std::set<IObserver>::iterator itr = observers.begin(); itr != observers.end(); itr++)
        {
            int z = 3;
            itr->CallMeBack("x","y",z); // <-- This is the part that's not working
        }
    }
};

// Has internal logic to handle the callback
class B : public IObserver
{
private:
    A myA;
public:
    B(A& a)
    {
        myA = a;
        myA .RegisterObserver(*this);
    }
    ~B()
    {
        myA .UnregisterObserver(*this);
    }

    int CallMeBack(const char* x, const char* y, int z)
    {
        return 0;
    }
};

Compiler Errors:

prog.cpp: In member function ‘void A::NotifyAll()’:
prog.cpp:38: error: passing ‘const IObserver’ as ‘this’ argument of ‘virtual int IObserver::CallMeBack(const char*, const char*, int)’ discards qualifiers
/usr/lib/gcc/i686-pc-linux-gnu/4.3.4/include/g++-v4/bits/stl_function.h: In member function ‘bool std::less<_Tp>::operator()(const _Tp&, const _Tp&) const [with _Tp = IObserver]’:
/usr/lib/gcc/i686-pc-linux-gnu/4.3.4/include/g++-v4/bits/stl_tree.h:1141:   instantiated from ‘std::pair<typename std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::iterator, bool> std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_insert_unique(const _Val&) [with _Key = IObserver, _Val = IObserver, _KeyOfValue = std::_Identity<IObserver>, _Compare = std::less<IObserver>, _Alloc = std::allocator<IObserver>]’
/usr/lib/gcc/i686-pc-linux-gnu/4.3.4/include/g++-v4/bits/stl_set.h:381:   instantiated from ‘std::pair<typename std::_Rb_tree<_Key, _Key, std::_Identity<_Key>, _Compare, typename _Alloc::rebind<_Key>::other>::const_iterator, bool> std::set<_Key, _Compare, _Alloc>::insert(const _Key&) [with _Key = IObserver, _Compare = std::less<IObserver>, _Alloc = std::allocator<IObserver>]’
prog.cpp:25:   instantiated from here
/usr/lib/gcc/i686-pc-linux-gnu/4.3.4/include/g++-v4/bits/stl_function.h:230: error: no match for ‘operator<’ in ‘__x < __y’

Please note that I do most of my programming in C#, so excuse my ignorance when it comes to the "nuances" of C++. Could anybody help me figure out what's the right way to implement this?

Update

Changed my code based on Nawaz's answer, here is the updated code: http://www.ideone.com/AH3KG

Upvotes: 0

Views: 150

Answers (3)

Mark B
Mark B

Reputation: 96281

The reason is that all objects in a set are immutable, so you can't call a mutating function (CallMeBack) on them.

Further, since you're storing the observers by value in your set they'll be sliced and never exhibit any polymorphic behavior.

If you make the set contain (smart depending on ownership) pointers, that solves both problems at once.

Upvotes: 2

Ben Voigt
Ben Voigt

Reputation: 283733

The only thing that you might not be able to fix easily just by reading the error messages is:

std::set<IObserver> observers;

This needs to be a collection of pointers. Making copies of the observer objects is almost certainly the wrong thing to do. Use

std::set<IObserver*> observers;

This will also fix the problem with less_than, because pointers can be compared even when the objects they point to don't define an ordering.

Here it is, fixed: http://ideone.com/9jzpK

Since you mentioned const-correctness, here's a variant in which the observers are const: http://ideone.com/gxsCo

Upvotes: 2

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361622

itr->("x","y",z);

What is this? You wanted to write this:

itr->CallMeBack("x","y",z);

Second, make B(const A& a) as:

 B(A& a);

That is, the parameter shouldn't be const. And other errors are too easy to recognize, for example, you're using a in the destructor of B, yet its not a member variable of the class. Make some effort in reading the error messages, and fix these error yourself.

Upvotes: 1

Related Questions