Regarding feedback on OpenWire C++ client

[Mats Forslöf <Ma...@portwise.com>]

Hi All,

Thanks everyone for looking at and testing the latest C++ client update. Sorry if we have not been so clear regarding the stability of the code, the code is highly untested and as said in the upload mail we will commence testing/debugging shortly - so expect bugs!!

As we've just finished the port we will shift focus and look into all the issues you have raised, see comments below.

1) Use of smart pointers.

Though the user interface would be cleaner without smart pointers they serves a purpose even when the passed in object is not owned by the client. As soon as an object is shared between two classes you have the problem when the object should be deleted - this problem is eliminated when using SP's, the object is deleted when both object releases its reference to it.

SP's and Strings: Semi-agreed :) The setters should all have "const char*" but we could change the getters to std:string (without SP), then it is up to the user to either use it as a std:string or a "const char*".

2) ITextMessage should extend IMessage

This was a last minute change to make it compile. Nate, your solution seems to be a viable way of handling it - thanks!

3) SP dynamic_cast rather than reinterpret_cast

It is correct that reinterpret_cast<> does not adjust the pointer value as needed when casting between different classes in a class hierarchy. However, the reinterpret_cast<> is used for casting between void* and objects and is needed for the p<> template to work. Where such pointer-value-adjustment is needed (up/down-casting), static_cast<> is used which works the same as dynamic_cast<> except that it does no runtime type check.

4) ExceptionListener

An ExceptionListener is missing. We could either add a method on IMessageListener or add a new interface IExceptionListener, prefer the latter. Ok?

5) .NET namespace

Hiram, what rules does Apache imply on this? Is it ok to use the namespaces your giving examples on? Looking on the Java code all Apache code starts with org.apache.

Regards,
Mats & David

Re: Regarding feedback on OpenWire C++ client

[Nathan Mittler <na...@gmail.com>]

Hi Mats,

On 3/27/06, Mats Forslöf <Ma...@portwise.com> wrote:
>
> Hi All,
>
> Thanks everyone for looking at and testing the latest C++ client update.
> Sorry if we have not been so clear regarding the stability of the code, the
> code is highly untested and as said in the upload mail we will commence
> testing/debugging shortly - so expect bugs!!


No problem, bugs are expected ... just reporting that I think I found one
:-)

As we've just finished the port we will shift focus and look into all the
> issues you have raised, see comments below.
>
> 1) Use of smart pointers.
>
> Though the user interface would be cleaner without smart pointers they
> serves a purpose even when the passed in object is not owned by the client.
> As soon as an object is shared between two classes you have the problem when
> the object should be deleted - this problem is eliminated when using SP's,
> the object is deleted when both object releases its reference to it.


I'm not sure I agree.  For user-generated objects, it should be up to the
user to add and remove references to them across the openwire library (such
as references to IMessageListeners).  When the user passes in a reference to
one of his objects, he's not expecting to give any amount of lifetime
control to the openwire code.  In fact, the user object may not even have
been created on the heap, it may be a local variable that was added just for
the scope of a function.  In this case, when the openwire library decides to
delete it, the application blows up!  We have to make a separation of what
is and what is not under our library's control.  We can't assume a structure
of the client code, this forces the client into a particular way of
programming and causes our client to become a burden to the developer and
will turn them off to using ActiveMQ.

SP's and Strings: Semi-agreed :) The setters should all have "const char*"
> but we could change the getters to std:string (without SP), then it is up to
> the user to either use it as a std:string or a "const char*".


I think symmetry is a good thing.  We should pick one way or the other ...
either both the getter and setter take const char* or they both take
strings.  Also, when passing strings around, they should be passed as "const
std::string&" ... this way you don't have to copy the entire string, you
just pass a constant reference to it.  This will save a lot of processing
when dealing with large text messages, for example.


2) ITextMessage should extend IMessage
>
> This was a last minute change to make it compile. Nate, your solution
> seems to be a viable way of handling it - thanks!


No problem.

3) SP dynamic_cast rather than reinterpret_cast
>
> It is correct that reinterpret_cast<> does not adjust the pointer value as
> needed when casting between different classes in a class hierarchy. However,
> the reinterpret_cast<> is used for casting between void* and objects and is
> needed for the p<> template to work. Where such pointer-value-adjustment is
> needed (up/down-casting), static_cast<> is used which works the same as
> dynamic_cast<> except that it does no runtime type check.


Hmmm ... so why are we using void* instead of T*?


4) ExceptionListener
>
> An ExceptionListener is missing. We could either add a method on
> IMessageListener or add a new interface IExceptionListener, prefer the
> latter. Ok?


Agreed.

5) .NET namespace
>
> Hiram, what rules does Apache imply on this? Is it ok to use the
> namespaces your giving examples on? Looking on the Java code all Apache code
> starts with org.apache.
>
> Regards,
> Mats & David
>

Regards,
Nate

Re: Regarding feedback on OpenWire C++ client

[Hiram Chirino <hi...@hiramchirino.com>]

On 3/27/06, Mats Forslöf <Ma...@portwise.com> wrote:
>
> 5) .NET namespace
>
> Hiram, what rules does Apache imply on this? Is it ok to use the namespaces your giving examples on? Looking on the Java code all Apache code starts with org.apache.
>

As far as I know the org.apache namespace is a Java thing as it's the
only language the encourages that form.  I've never seen this form use
in other languages yet.

> Regards,
> Mats & David
>


--
Regards,
Hiram