From 83b655496f8bafc98aaf5ece58b065cd107b1db3 Mon Sep 17 00:00:00 2001 From: tchernobog Date: Fri, 15 Sep 2006 09:34:12 +0000 Subject: [PATCH] - Use a NotifyLock instead of (History|Simulation)::set_notify_enabled() method, which is more elegant and also exception-safe - Delete set_notify_enabled() method from ConcreteHistory; it was both wrong and useless, and caused impredictable behaviour! - Don't make some methods of History and Simulation virtual if we don't want the user to override them - Loading from file and jumping to an instant of the simulation should be much quickier now git-svn-id: svn://svn.gna.org/svn/sgpemv2/trunk@1170 3ecf2c5c-341e-0410-92b4-d18e462d057c --- plugins/xmlsave/src/xml_serializer.cc | 4 +-- src/add_request_dialog.cc | 4 +-- src/backend/concrete_history.cc | 8 ----- src/backend/concrete_history.hh | 2 -- src/backend/concrete_simulation.cc | 26 +++----------- src/backend/concrete_simulation.hh | 1 - src/backend/history.cc | 13 +++++-- src/backend/sgpemv2/history.hh | 50 +++++++++++++++++++-------- src/backend/sgpemv2/simulation.hh | 39 ++++++++++++++++----- src/backend/simulation.cc | 12 +++++-- src/jump_to_dialog.cc | 8 ++--- 11 files changed, 94 insertions(+), 73 deletions(-) diff --git a/plugins/xmlsave/src/xml_serializer.cc b/plugins/xmlsave/src/xml_serializer.cc index a6515ec..b7fa074 100644 --- a/plugins/xmlsave/src/xml_serializer.cc +++ b/plugins/xmlsave/src/xml_serializer.cc @@ -70,7 +70,7 @@ void XMLSerializer::restore_snapshot(const Glib::ustring& filename, History& his // DEBUG - remove me when finished // disable notifications over history until the end of this method - bool was_enabled = hist.set_notify_enabled(false); + History::LockNotify h_lock(hist); #ifdef LIBXML_SAX1_ENABLED xmlDocPtr doc; @@ -107,8 +107,6 @@ void XMLSerializer::restore_snapshot(const Glib::ustring& filename, History& his #error Compilation of LIBXML with SAX1 support must be enabled #endif /* LIBXML_SAX1_ENABLED */ - // Re-enable notifications over history observers - hist.set_notify_enabled(was_enabled); } diff --git a/src/add_request_dialog.cc b/src/add_request_dialog.cc index b1198d0..761b330 100644 --- a/src/add_request_dialog.cc +++ b/src/add_request_dialog.cc @@ -166,7 +166,7 @@ AddRequestDialog::run_edit(Request& request) { assert(_list_model->children()); - bool was_enabled = history.set_notify_enabled(false); + History::LockNotify h_lock(history); // I know it's a bit hack-ish, but do you know an elegant alternative way? for(Iseq::iterator> it = iseq(subrequests); it; ++it) @@ -178,8 +178,6 @@ AddRequestDialog::run_edit(Request& request) for(Iseq it = iseq(sreq_container); it; ++it) history.add_subrequest(request, (*it)[_list_key_column], (*it)[_list_duration_column]); - - history.set_notify_enabled(was_enabled); } hide(); diff --git a/src/backend/concrete_history.cc b/src/backend/concrete_history.cc index db21e73..ddd6423 100644 --- a/src/backend/concrete_history.cc +++ b/src/backend/concrete_history.cc @@ -523,14 +523,6 @@ ConcreteHistory::reset(bool notify) notify_change(); } -void -ConcreteHistory::notify_change() -{ - History::RegisteredObservers::iterator it; - for (it = _observers.begin(); it != _observers.end(); it++) - (*it)->update(*this); -} - bool ConcreteHistory::is_sealed() const diff --git a/src/backend/concrete_history.hh b/src/backend/concrete_history.hh index d9865c6..db699bc 100644 --- a/src/backend/concrete_history.hh +++ b/src/backend/concrete_history.hh @@ -124,8 +124,6 @@ namespace sgpem typedef std::vector Snapshots; Snapshots _snapshots; - virtual void notify_change(); - private: // Disable assignment, implement it only if needed ConcreteHistory& operator=(const ConcreteHistory& op2); diff --git a/src/backend/concrete_simulation.cc b/src/backend/concrete_simulation.cc index 23ca660..1041204 100644 --- a/src/backend/concrete_simulation.cc +++ b/src/backend/concrete_simulation.cc @@ -73,8 +73,8 @@ ConcreteSimulation::jump_to(History::position p) throw(UserInterruptException, N // Disable momentarily updates for registered observers on // sgpem::Simulation and sgpem::History. - bool his_enabled = _history.set_notify_enabled(false); - bool sim_enabled = set_notify_enabled(false); + History::LockNotify h_lock(_history); + Simulation::LockNotify s_lock(*this); pause(); @@ -82,30 +82,12 @@ ConcreteSimulation::jump_to(History::position p) throw(UserInterruptException, N History::position increment = 0; while (yet_to_finish && p > _history.get_front() + increment) { - try - { - yet_to_finish = step(); - increment++; - } - catch(const CPUPolicyException&) - { - // Lookout that notifications have to be re-enabled. - _history.set_notify_enabled(his_enabled); - set_notify_enabled(sim_enabled); - throw; - } + yet_to_finish = step(); + increment++; } get_history().set_front(std::min(p, _history.get_size())); if (!yet_to_finish) stop(); - - // Reenables updates to registered observers. - // Calls _history.notify_change() on reactivation. - _history.set_notify_enabled(his_enabled); - - // Do the same for notifications on the state - // of Simulation - set_notify_enabled(sim_enabled); } diff --git a/src/backend/concrete_simulation.hh b/src/backend/concrete_simulation.hh index a331555..8458424 100644 --- a/src/backend/concrete_simulation.hh +++ b/src/backend/concrete_simulation.hh @@ -81,6 +81,5 @@ namespace sgpem } - #endif diff --git a/src/backend/history.cc b/src/backend/history.cc index cf0c71c..574d000 100644 --- a/src/backend/history.cc +++ b/src/backend/history.cc @@ -83,8 +83,15 @@ History::set_notify_enabled(bool enabled) return old_value; } -bool -History::is_notify_enabled() const + +// --------- History::LockNotify --------------- + +History::LockNotify::LockNotify(History& history) + : _h(history), _was_enabled(_h.set_notify_enabled(false)) { - return _notify; +} + +History::LockNotify::~LockNotify() +{ + _h.set_notify_enabled(_was_enabled); } diff --git a/src/backend/sgpemv2/history.hh b/src/backend/sgpemv2/history.hh index 545cf5e..100bd93 100644 --- a/src/backend/sgpemv2/history.hh +++ b/src/backend/sgpemv2/history.hh @@ -56,6 +56,10 @@ namespace sgpem class SG_DLLEXPORT History { public: + // Forward declaration + class LockNotify; + friend class History::LockNotify; + typedef unsigned int size_t; typedef unsigned int time_t; typedef unsigned int position; @@ -134,6 +138,14 @@ namespace sgpem virtual void attach(HistoryObserver& observer); virtual void detach(const HistoryObserver& observer); + virtual void reset() = 0; + + protected: + typedef std::vector RegisteredObservers; + RegisteredObservers _observers; + + void notify_change(); + /** \brief Enable/disable notifications to registered observers * * This is quite useful to disable momentarily notification while you @@ -142,26 +154,36 @@ namespace sgpem * * \return The old value */ - virtual bool set_notify_enabled(bool enabled = true); - bool is_notify_enabled() const; - - virtual void reset() = 0; - - protected: - typedef std::vector RegisteredObservers; - RegisteredObservers _observers; - - virtual void notify_change(); + bool set_notify_enabled(bool enabled = true); position _front; private: - - bool _notify; - } - ; //~ class History + }; //~ class History + + /** \brief Disables notifications to History during the life of this object + * + * This class is useful if you've to do a lot of sequential operations on + * History that would reset it / notify its observers. For example, when loading + * from a file. In this case, create an object of this type on the stack. + * The destructor will take care of re-enabling notifications. + */ + class SG_DLLEXPORT History::LockNotify + { + public: + LockNotify(History& history); + ~LockNotify(); + + private: + History& _h; + bool _was_enabled; + + LockNotify(const LockNotify&); + LockNotify& operator=(const LockNotify&); + }; //~ class History::LockNotify + }//~ namespace sgpem diff --git a/src/backend/sgpemv2/simulation.hh b/src/backend/sgpemv2/simulation.hh index e11b270..69275b0 100644 --- a/src/backend/sgpemv2/simulation.hh +++ b/src/backend/sgpemv2/simulation.hh @@ -67,6 +67,9 @@ namespace sgpem class SG_DLLEXPORT Simulation : public Singleton { public: + class LockNotify; + friend class Simulation::LockNotify; + enum state { state_running = 0xdeafd0d0, @@ -170,6 +173,10 @@ namespace sgpem virtual void attach(SimulationObserver& observer); virtual void detach(const SimulationObserver& observer); + protected: + typedef std::vector RegisteredObservers; + RegisteredObservers _observers; + /** \brief Enable/disable notifications to registered observers * * This is quite useful to disable momentarily notification while you @@ -178,20 +185,36 @@ namespace sgpem * * \return The old value */ - virtual bool set_notify_enabled(bool enabled = true); - bool is_notify_enabled() const; - - protected: - typedef std::vector RegisteredObservers; - RegisteredObservers _observers; + bool set_notify_enabled(bool enabled = true); Simulation(); // Constructor - virtual void notify_change(); + void notify_change(); private: bool _notify; - }; + }; //~ class Simulation + + /** \brief Disables notifications to Simulation during the life of this object + * + * This class is useful if you've to do a lot of sequential operations on + * Simulation that would reset it / notify its observers. For example, when loading + * from a file. In this case, create an object of this type on the stack. + * The destructor will take care of re-enabling notifications. + */ + class SG_DLLEXPORT Simulation::LockNotify + { + public: + LockNotify(Simulation& simulation); + ~LockNotify(); + + private: + Simulation& _s; + bool _was_enabled; + + LockNotify(const LockNotify&); + LockNotify& operator=(const LockNotify&); + }; //~ class Simulation::LockNotify } diff --git a/src/backend/simulation.cc b/src/backend/simulation.cc index 43b6730..57b83b4 100644 --- a/src/backend/simulation.cc +++ b/src/backend/simulation.cc @@ -89,8 +89,14 @@ Simulation::set_notify_enabled(bool enabled) return old_value; } -bool -Simulation::is_notify_enabled() const +// --------- Simulation::LockNotify --------------- + +Simulation::LockNotify::LockNotify(Simulation& simulation) + : _s(simulation), _was_enabled(_s.set_notify_enabled(false)) { - return _notify; +} + +Simulation::LockNotify::~LockNotify() +{ + _s.set_notify_enabled(_was_enabled); } diff --git a/src/jump_to_dialog.cc b/src/jump_to_dialog.cc index 088be5a..6f111d5 100644 --- a/src/jump_to_dialog.cc +++ b/src/jump_to_dialog.cc @@ -84,9 +84,6 @@ JumpToDialog::start() // start listening to simulation updates sim.attach(*this); - // remember state of notifications for History - bool reenable = h.is_notify_enabled(); - try { if(_target_instant < h.get_size() - 1) @@ -94,9 +91,9 @@ JumpToDialog::start() else sim.jump_to(h.get_size() - 1); - // disable notifications since we could call + // disable notifications on History since we could call // run() a lot of times - h.set_notify_enabled(false); + History::LockNotify h_lock(h); while(h.get_front() <= _target_instant) { sim.run(); @@ -149,7 +146,6 @@ JumpToDialog::start() // Ending successfully: detach me, reenable notifications, // and emit response ``okay'' sim.detach(*this); - h.set_notify_enabled(reenable); response(Gtk::RESPONSE_OK); }