Merge branch 'issue/cleanup_observer' into 'master'

Simplify observer

See merge request !131

src/amdis/AdaptiveGrid.hpp

@@ -27,27 +27,6 @@
 
 namespace AMDiS
 {
-  namespace event
-  {
-    /** Event generated from an AdaptiveGrid when calling preAdapt(). It contains the return value
-     *  of preAdapt() as its 'mightCoarsen' member and is passed to registered observers after
-     *  calling preAdapt on the underlying grid.
-     **/
-    struct preAdapt { bool mightCoarsen; };
-
-    /** Event generated from an AdaptiveGrid when calling adapt(). Its 'adapted' member contains the
-     *  value true if either preAdapt() or adapt() returned true. This event is passed to registered
-     *  observers after calling adapt on the underlying grid.
-     **/
-    struct adapt { bool adapted; };
-
-    /** Event generated from an AdaptiveGrid when calling postAdapt().This event is passed to
-     *  registered observers after calling postAdapt on the underlying grid.
-     **/
-    struct postAdapt {};
-  }
-
-
   // forward declaration
   template <class HostGrid>
   class AdaptiveGridFamily;
@@ -65,7 +44,7 @@ namespace AMDiS
   class AdaptiveGrid
       : public Dune::GridDefaultImplementation<
           HG::dimension, HG::dimensionworld, typename HG::ctype, AdaptiveGridFamily<HG> >
-      , public Signals<event::preAdapt, event::adapt, event::postAdapt>
+      , public Notifier<event::preAdapt, event::adapt, event::postAdapt>
   {
     using Self = AdaptiveGrid<HG>;
 

src/amdis/DOFVector.hpp

@@ -28,12 +28,6 @@ namespace AMDiS
   template <class PB>
   class ParallelGlobalBasis;
 
-  namespace event
-  {
-    struct preAdapt;
-    struct postAdapt;
-    template <class B> struct basisUpdate;
-  }
 
   /// \brief The basic container that stores a base vector and a corresponding basis
   /**
@@ -43,11 +37,12 @@ namespace AMDiS
   template <class GB, class T = double>
   class DOFVector
       : public VectorBase<GB, VectorBackend<BackendTraits<GB,T>>>
-      , public Observer<GB, event::preAdapt, event::basisUpdate<GB>, event::postAdapt>
+      , private Observer<event::preAdapt>
+      , private Observer<event::adapt>
+      , private Observer<event::postAdapt>
   {
     using Self = DOFVector;
     using Super = VectorBase<GB, VectorBackend<BackendTraits<GB,T>>>;
-    using Obs = Observer<GB, event::preAdapt, event::basisUpdate<GB>, event::postAdapt>;
 
   public:
     using Backend = VectorBackend<BackendTraits<GB,T>>;
@@ -68,7 +63,9 @@ namespace AMDiS
     DOFVector(std::shared_ptr<GB> const& basis,
               DataTransferOperation op = DataTransferOperation::INTERPOLATE)
       : Super(basis)
-      , Obs(basis)
+      , Observer<event::preAdapt>(basis->gridView().grid())
+      , Observer<event::adapt>(*basis)
+      , Observer<event::postAdapt>(basis->gridView().grid())
       , dataTransfer_(op, basis)
       , basis_(basis)
     {}
@@ -165,24 +162,27 @@ namespace AMDiS
       setDataTransfer(newDataTransfer(op, basis_));
     }
 
+  protected:
+
     /// Override of Observer update(event::preAdapt) method. Redirects to preAdapt method of the
     /// \ref DataTransfer object.
-    void update(event::preAdapt const& e) override
+    void updateImpl(event::preAdapt e) override
     {
-      dataTransfer_.preAdapt(*this, e.mightCoarsen);
+      dataTransfer_.preAdapt(*this, e.value);
     }
 
     /// Override of Observer update(event::adapt) method. Redirects to adapt method of the
     /// \ref DataTransfer object.
-    void update(event::basisUpdate<GB> const&) override
+    void updateImpl(event::adapt e) override
     {
+      assert(e.value);
       this->resize();
       dataTransfer_.adapt(*this);
     }
 
     /// Override of Observer update(event::postAdapt) method. Redirects to postAdapt method of the
     /// \ref DataTransfer object.
-    void update(event::postAdapt const&) override
+    void updateImpl(event::postAdapt) override
     {
       dataTransfer_.postAdapt(*this);
     }

src/amdis/Observer.hpp

@@ -12,158 +12,34 @@
 
 namespace AMDiS
 {
-  namespace Impl
+  namespace event
   {
-    // forward declaration
-    template <class Event>
-    class ObserverInterface;
+    /** 
+     * An event that is signaled before the actual adaption happens. Example: grid.preAdapt(). 
+     * The \ref value might indicate whether any pre-processing is necessary.
+     **/
+    struct preAdapt { bool value = true; };
 
-    template <class Event>
-    class SignalBase
-    {
-    public:
-      /// Attaches an observer to this class. This method will be called by all observers with
-      /// with themselves as argument.
-      void attachObserver(ObserverInterface<Event>* o) const
-      {
-        observers_.push_back(o);
-      }
-
-      /// Detaches an observer to this class. This method will be called by all observers with
-      /// with themselves as argument.
-      void detachObserver(ObserverInterface<Event>* o) const
-      {
-        auto it = std::find(observers_.begin(), observers_.end(), o);
-        if (it != observers_.end())
-          observers_.erase(it);
-      }
-
-      /// Notify all observers that have called attachObserver but have not called detachObserver
-      void notify(Event const& e) const
-      {
-        for (ObserverInterface<Event>* o : observers_)
-          o->update(e);
-      }
-
-    private:
-      /// List of observers that need to be notified in case of an event
-      // NOTE: this list is mutable, since the notification list itself is not part
-      //       of the internal state of the object signaling the event.
-      mutable std::list<ObserverInterface<Event>*> observers_;
-    };
-
-
-    template <class Event>
-    class ObserverInterface
-    {
-    public:
-      virtual ~ObserverInterface() = default;
-
-      /// Attach the observer to a subject. It will then receive notifications from the subject when
-      /// an event of type Event it triggered.
-      void attach(std::shared_ptr<SignalBase<Event> const> const& subject)
-      {
-        if (bool(subject))
-          subject->attachObserver(this);
-      }
-
-      /// Detach the observer from the subject. It will then no longer receive notifications. This
-      /// must be called before the observer is deleted.
-      void detach(std::shared_ptr<SignalBase<Event> const> const& subject)
-      {
-        if (bool(subject))
-          subject->detachObserver(this);
-      }
-
-      /// This method will be called by the subject when triggering the event.
-      virtual void update(Event const&)
-      {
-        error_exit("Method must be overridden by derived class");
-      }
-    };
-
-
-    template <class Event>
-    class ObserverBase
-        : virtual public ObserverInterface<Event>
-    {
-      using Self = ObserverBase;
-
-    private:
-      ObserverBase() = default;
-
-      // use subject if Event is handled directly
-      template <class S,
-        REQUIRES(std::is_base_of<SignalBase<Event>, S>::value)>
-      ObserverBase(std::shared_ptr<S const> subject, Dune::PriorityTag<2>)
-        : subject_(std::move(subject))
-      {
-        this->attach(subject_);
-      }
-
-      // get next subject in hierarchy if Event is not handled
-      template <class Observer,
-        class = void_t<decltype(std::declval<Observer>().subject_)> >
-      ObserverBase(std::shared_ptr<Observer const> const& o, Dune::PriorityTag<1>)
-        : ObserverBase(o->subject_)
-      {}
-
-      // non-observable type
-      template <class T>
-      ObserverBase(std::shared_ptr<T const> const& other, Dune::PriorityTag<0>)
-      {
-        /* fallback implementation */
-      }
-
-    public:
-      template <class T>
-      ObserverBase(std::shared_ptr<T const> const& arg)
-        : ObserverBase(arg, Dune::PriorityTag<42>{})
-      {}
-
-      /// Copy constructor
-      ObserverBase(Self const& that)
-        : subject_(that.subject_)
-      {
-        this->attach(subject_);
-      }
-
-      /// Move constructor
-      ObserverBase(Self&& that)
-      {
-        swap(*this, that);
-      }
-
-      /// Destructor. Detaches observer from the subject
-      ~ObserverBase() override
-      {
-        this->detach(subject_);
-      }
-
-      /// Copy/Move assignment
-      Self& operator=(Self that)
-      {
-        swap(*this, that);
-        return *this;
-      }
-
-      /// Swaps the contents of lhs and rhs
-      friend void swap(Self& lhs, Self& rhs)
-      {
-        using std::swap;
-        lhs.detach(lhs.subject_);
-        rhs.detach(rhs.subject_);
-        swap(lhs.subject_, rhs.subject_);
-        lhs.attach(lhs.subject_);
-        rhs.attach(rhs.subject_);
-      }
-
-    private:
-      /// The observed subject
-      std::shared_ptr<SignalBase<Event> const> subject_ = nullptr;
-    };
-
-  } // end namespace Impl
+    /** 
+     * An event that is called directly of the adaption. Example: grid.adapt().
+     * The \ref value indicates whether something is changed during adaption.
+     **/
+    struct adapt { bool value = true; };
+
+    /** 
+     * An event that is called after adaption to indicate the start of a clean-up phase.
+     **/
+    struct postAdapt {};
+  }
+
+
+  template <class Event>
+  class ObserverInterface
+  {
+  public:
+    virtual ~ObserverInterface() = default;
+    virtual void update(Event e) = 0;
+  };
 
 
   /// \brief Mixin for signaling of certain events.
@@ -176,57 +52,110 @@ namespace AMDiS
    *      to the instance of T (see Observer)
    */
   template <class Event, class... Events>
-  class Signals
-      : public Impl::SignalBase<Event>
-      , public Signals<Events...>
+  class Notifier
+      : public Notifier<Event>
+      , public Notifier<Events...>
   {
   public:
-    using Impl::SignalBase<Event>::notify;
-    using Signals<Events...>::notify;
+    using Notifier<Event>::notify;
+    using Notifier<Events...>::notify;
   };
 
   template <class Event>
-  class Signals<Event>
-      : public Impl::SignalBase<Event>
+  class Notifier<Event>
   {
   public:
-    using Impl::SignalBase<Event>::notify;
+    /// Call the \ref update method on all attached observers.
+    void notify(Event const& e)
+    {
+      for (ObserverInterface<Event>* o : observers_)
+        o->update(e);
+    }
+
+    /// Attach a new observer that gets called on \ref notify
+    void attach(ObserverInterface<Event>* o)
+    {
+      observers_.push_back(o);
+    }
+
+    /// Detaches the passed observer from the list, if stored.
+    void detach(ObserverInterface<Event>* o)
+    {
+      auto it = std::find(observers_.begin(), observers_.end(), o);
+      if (it != observers_.end())
+        observers_.erase(it);
+    }
+
+  private:
+    std::list<ObserverInterface<Event>*> observers_;
   };
 
 
-  /// \brief Mixin for reacting to certain events.
-  /**
-   *  Derived classes can react to events by implementing the functions `update(Event)` for
-   *  all Events specified in the template parameter list, whose origin is an instance of class
-   *  `Subject` specified in the constructor to Observer and any indirectly observed class instances.
-   *  Observed instances may include:
-   *  - the instance of class `Subject` passed to the constructor
-   *  - the instance passed to the constructor of `Subject::Observer<S>` if Subject inherits from
-   *    \ref Observer<S>
-   *  - all other instances indirectly accessible in the above way
-   *
-   *  For each event E the first object in the above hierarchy that implements \ref Signals<Es...> with E
-   *  included in Es... will be observed by this class.
-   */
-  template <class Subject, class... Events>
+  /// Implementation of the \ref ObserverInterface 
+  template <class Event, class... Tags>
   class Observer
-      : public Impl::ObserverBase<Events>...
+      : public ObserverInterface<Event>
   {
-    template <class E>
-    friend class Impl::ObserverBase;
-
   public:
-    Observer(std::shared_ptr<Subject const> const& s)
-      : Impl::ObserverBase<Events>(s)...
-      , subject_(std::move(s))
-    {}
+    template <class Notifier>
+    Observer(Notifier const& notifier)
+      : notifier_(const_cast<Notifier*>(&notifier))
+    {
+      notifier_->attach(this);
+    }
+
+    /// Destructor, detaches from the notifier
+    virtual ~Observer()
+    {
+      assert(notifier_);
+      notifier_->detach(this);
+    }
+
+    /// Copy constructor. Attaches this to the copied notifier
+    Observer(Observer const& other)
+      : notifier_(other.notifier_)
+    {
+      notifier_->attach(this);
+    }
+
+    /// Copy-assignment operator, copies the notifier and attaches this.
+    Observer& operator=(Observer const& other)
+    {
+      notifier_ = other.notifier_;
+      notifier_->attach(this);
+      return *this;
+    }
+
+    /// Move assignment operator, implemented in terms of the move
+    /// assignment operator
+    Observer(Observer&& other)
+    {
+      *this = std::move(other);
+    }
+
+    /// Move-assignment operator, copies the notifier and attaches this.
+    Observer& operator=(Observer&& other)
+    {
+      notifier_ = other.notifier_;
+      notifier_->attach(this);
+      return *this;
+    }
+
+    /// Implementation of the interface method \ref ObserverInterface::update.
+    /// Redirects to the \ref updateImpl method with additional \ref Tags parameters
+    void update(Event e) final
+    {
+      updateImpl(e, Tags{}...);
+    }
 
-    Observer(std::shared_ptr<Subject> const& s)
-      : Observer(std::const_pointer_cast<Subject const>(s))
-    {}
+  protected:
+    /// \brief Implementation of the update method in derived class
+    // NOTE: The additional `Tags...` arguments can be used to distinguish
+    //       between multiple observers of the same event.
+    virtual void updateImpl(Event e, Tags...) = 0;
 
   private:
-    std::shared_ptr<Subject const> subject_ = nullptr;
+    Notifier<Event>* notifier_ = nullptr;
   };
 
 } // end namespace AMDiS

src/amdis/functions/ParallelGlobalBasis.hpp

@@ -45,16 +45,6 @@ namespace Dune
 
 namespace AMDiS
 {
-  namespace event
-  {
-    template <class B>
-    struct basisUpdate
-    {
-      using Basis = B;
-      Basis const& basis;
-    };
-  }
-
   /**
    * \brief Parallel global basis defined on a (sequential) pre-basis
    *
@@ -65,15 +55,11 @@ namespace AMDiS
    */
   template <class PB>
   class ParallelGlobalBasis
-      : public Observer<typename PB::GridView::Grid, event::adapt>
-      , public Signals<event::basisUpdate<ParallelGlobalBasis<PB>>>
+      : public Notifier<event::adapt>
+      , private Observer<event::adapt>
   {
-    using Subject = typename PB::GridView::Grid;
-    using Super = Observer<Subject, event::adapt>;
     using Self = ParallelGlobalBasis<PB>;
 
-    struct DummyImpl {};
-
   public:
 
     /// Pre-basis providing the implementation details
@@ -109,6 +95,7 @@ namespace AMDiS
     /// Type of the communicator
     using Comm = typename BackendTraits<Self>::Comm;
 
+    struct DummyImpl {};
     using ADH = Dune::AdaptDataHandle<Grid, DummyImpl>;
 
 
@@ -124,7 +111,7 @@ namespace AMDiS
     template <class... Args,
       Dune::Functions::enableIfConstructible<PreBasis, Args...> = 0>
     ParallelGlobalBasis(std::string const& name, Grid const& grid, Args&&... args)
-      : Super(Dune::stackobject_to_shared_ptr(grid))
+      : Observer<event::adapt>(grid)
       , preBasis_(FWD(args)...)
     {
       static_assert(Dune::models<Dune::Functions::Concept::PreBasis<GridView>, PreBasis>(),
@@ -172,16 +159,6 @@ namespace AMDiS
       return preBasis_;
     }
 
-    /// Updates the underlying basis when event::adapt is triggered by the observed grid
-    void update(event::adapt const& e) override
-    {
-      if (e.adapted) {
-        update(gridView());
-        event::basisUpdate<Self> eNew{*this};
-        this->notify(eNew);
-      }
-    }
-
     /// \brief Update the stored grid view
     /**
      * This will update the indexing information of the global basis as well as the communicator.
@@ -245,6 +222,17 @@ namespace AMDiS
       return ADH{};
     }
 
+  protected:
+
+    /// Updates the underlying basis when event::adapt is triggered by the observed grid
+    void updateImpl(event::adapt e) override
+    {
+      if (e.value) {
+        update(gridView());
+        this->notify(e);
+      }
+    }
+
   protected:
     PreBasis preBasis_;
     PrefixPath prefixPath_ = {};

test/ObserverTest.cpp

@@ -6,7 +6,7 @@
 
 #include "Tests.hpp"
 
-using namespace AMDiS;
+namespace AMDiS {
 
 namespace event {
   struct fooEvent {};
@@ -14,6 +14,8 @@ namespace event {
   struct bazEvent {};
 }
 
+} // end namespace AMDiS
+
 std::string testValue = "";
 
 bool checkAndResetTestValue(std::string testName, std::string ref, std::string refOpt = "X")
@@ -23,8 +25,10 @@ bool checkAndResetTestValue(std::string testName, std::string ref, std::string r
   return result;
 }
 
+using namespace AMDiS;
+
 class A
-    : public Signals<event::fooEvent, event::barEvent, event::bazEvent>
+    : public Notifier<event::fooEvent, event::barEvent, event::bazEvent>
 {
 public:
   void foo()
@@ -53,43 +57,52 @@ public:
 };
 
 class B
-    : public Observer<A, event::fooEvent, event::bazEvent>
-    , public Signals<event::bazEvent>
+    : private Observer<event::fooEvent>
+    , private Observer<event::bazEvent>
+    , public Notifier<event::bazEvent>
 {
 public:
   B(std::shared_ptr<A> a)
-    : Observer<A, event::fooEvent, event::bazEvent>(a)
+    : Observer<event::fooEvent>(*a)
+    , Observer<event::bazEvent>(*a)
+    , a_(a)
   {}
 
-  void update(event::fooEvent const& e) override
+  void updateImpl(event::fooEvent) override
   {
     std::cout << "B::update(foo)\n";
     testValue += "B.foo ";
   }
 
-  void update(event::bazEvent const& e) override
+  void updateImpl(event::bazEvent e) override
   {
     std::cout << "B::update(baz)\n";
     testValue += "B.baz ";
     this->notify(e);
   }
+
+  std::shared_ptr<A> const& a() const { return a_; }
+
+  std::shared_ptr<A> a_;
 };
 
 class C
-    : public Observer<B, event::barEvent, event::bazEvent>
+    : private Observer<event::barEvent>
+    , private Observer<event::bazEvent>
 {
 public:
   C(std::shared_ptr<B> b)
-    : Observer<B, event::barEvent, event::bazEvent>(b)
+    : Observer<event::barEvent>(*b->a())
+    , Observer<event::bazEvent>(*b)
   {}
 
-  void update(event::barEvent const& e) override
+  void updateImpl(event::barEvent) override
   {
     std::cout << "C::update(bar)\n";
     testValue += "C.bar ";
   }
 
-  void update(event::bazEvent const& e) override
+  void updateImpl(event::bazEvent) override
   {
     std::cout << "C::update(baz)\n";
     testValue += "C.baz ";
@@ -97,7 +110,7 @@ public:
 };
 
 class D
-    : public Signals<event::barEvent>
+    : public Notifier<event::barEvent>
 {
 public:
   void bar()
@@ -110,22 +123,22 @@ public:
 };