> On 2012-01-20 19:14:44, Andrew Stitcher wrote: > > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/ConnectionObservers.h, line > > 34 > > <https://reviews.apache.org/r/3570/diff/1/?file=70162#file70162line34> > > > > I don't think that the class "ConnectionObservers" IS-A > > "ConnectionObserver" itself it seems to me that it has-a collection of > > "ConnectionObserver"s and delegates to them. > > > > I think you should completely remove the inheritance relationship.
The thinking is that a ConnectionObserver is a thing that is notified of connection events. The ConnectionObservers is a thing that is notified of events. Its implementation is to delegate to a collection of other ConnectionObservers but that's of no concern to the caller. - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3570/#review4503 ----------------------------------------------------------- On 2012-01-20 17:47:07, Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3570/ > ----------------------------------------------------------- > > (Updated 2012-01-20 17:47:07) > > > Review request for qpid, Gordon Sim and Kenneth Giusti. > > > Summary > ------- > > > This commit introduces a ConnectionObserver, needed by the HA code to get > notified of connections opening. > I also took the LinkRegistry::notify* calls out of Connection and refactored > LinkRegistry to use a ConnectionObserver. > > Do the ConnectionObserver changes this look good to put on trunk? > > > Diffs > ----- > > /branches/qpid-3603-2/qpid/cpp/src/Makefile.am 1233690 > /branches/qpid-3603-2/qpid/cpp/src/ha.mk 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/Broker.h 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/Connection.cpp 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/ConnectionObserver.h > PRE-CREATION > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/ConnectionObservers.h > PRE-CREATION > /branches/qpid-3603-2/qpid/cpp/src/qpid/broker/LinkRegistry.cpp 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/ha/ConnectionExcluder.h > PRE-CREATION > /branches/qpid-3603-2/qpid/cpp/src/qpid/ha/HaBroker.h 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/ha/HaBroker.cpp 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/ha/HaPlugin.cpp 1233690 > /branches/qpid-3603-2/qpid/cpp/src/qpid/ha/Settings.h 1233690 > /branches/qpid-3603-2/qpid/cpp/src/tests/ha_tests.py 1233690 > > Diff: https://reviews.apache.org/r/3570/diff > > > Testing > ------- > > > Thanks, > > Alan > >
