> On 2011-08-17 14:28:37, Ted Ross wrote:
> > trunk/qpid/cpp/include/qmf/EventNotifier.h, line 40
> > <https://reviews.apache.org/r/1557/diff/1/?file=33104#file33104line40>
> >
> >     The session arguments should be passed as references, not pointers.  
> > Pointers can be null and you will have to deal with that case whereas 
> > references cannot be null and are generally cleaner and more elegant.

The problem there is that the EventNotifier will have either a ConsoleSession 
or an AgentSession and not both, something will necessarily be null. That's why 
I chose a pointer instead of a reference.


> On 2011-08-17 14:28:37, Ted Ross wrote:
> > trunk/qpid/cpp/src/qmf/AgentSessionImpl.h, line 90
> > <https://reviews.apache.org/r/1557/diff/1/?file=33108#file33108line90>
> >
> >     Again, please use references rather than pointers.

Agreed here. I'll update the set and APIs on the Sessions to use a reference to 
an EventNotifier.


> On 2011-08-17 14:28:37, Ted Ross wrote:
> > trunk/qpid/cpp/src/qmf/EventNotifierImpl.cpp, line 54
> > <https://reviews.apache.org/r/1557/diff/1/?file=33113#file33113line54>
> >
> >     What is this doing here?

Sorry, leftover from debugging. I'll remove it.


- Darryl


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1557/#review1495
-----------------------------------------------------------


On 2011-08-17 14:07:58, Darryl Pierce wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1557/
> -----------------------------------------------------------
> 
> (Updated 2011-08-17 14:07:58)
> 
> 
> Review request for qpid, Kenneth Giusti, michael goulish, and Ted Ross.
> 
> 
> Summary
> -------
> 
> Provides a new method for providing notification to an interested party when 
> new messages are received.
> 
> The EventNotifier class can be associated with either a console or agent 
> session. The object provides a file descriptor which then becomes readable 
> when there are messages to be processed.
> 
> This implementation only supports Posix. There is some work necessary to get 
> a Windows implementation in place.
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/bindings/qmf2/examples/cpp/Makefile.am 1158370 
>   trunk/qpid/cpp/bindings/qmf2/examples/cpp/event_driven_list_agents.cpp 
> PRE-CREATION 
>   trunk/qpid/cpp/include/qmf/AgentSession.h 1158370 
>   trunk/qpid/cpp/include/qmf/ConsoleSession.h 1158370 
>   trunk/qpid/cpp/include/qmf/EventNotifier.h PRE-CREATION 
>   trunk/qpid/cpp/src/CMakeLists.txt 1158370 
>   trunk/qpid/cpp/src/qmf.mk 1158370 
>   trunk/qpid/cpp/src/qmf/AgentSession.cpp 1158370 
>   trunk/qpid/cpp/src/qmf/AgentSessionImpl.h PRE-CREATION 
>   trunk/qpid/cpp/src/qmf/ConsoleSession.cpp 1158370 
>   trunk/qpid/cpp/src/qmf/ConsoleSessionImpl.h 1158370 
>   trunk/qpid/cpp/src/qmf/EventNotifier.cpp PRE-CREATION 
>   trunk/qpid/cpp/src/qmf/EventNotifierImpl.h PRE-CREATION 
>   trunk/qpid/cpp/src/qmf/EventNotifierImpl.cpp PRE-CREATION 
>   trunk/qpid/cpp/src/qmf/PosixEventNotifierImpl.cpp PRE-CREATION 
>   trunk/qpid/cpp/src/tests/EventNotifierTest.cpp PRE-CREATION 
>   trunk/qpid/cpp/src/tests/Makefile.am 1158370 
> 
> Diff: https://reviews.apache.org/r/1557/diff
> 
> 
> Testing
> -------
> 
> An example agent takes the existing list_agents and uses an EventNotifier to 
> respond to incoming messages rather than blocking on the 
> ConsoleSession.nextReceiver() API.
> 
> A unit test verifies that the file handle provides by the EventNotifier type 
> is properly updating on incoming messgaes.
> 
> 
> Thanks,
> 
> Darryl
> 
>

Reply via email to