activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nathan Mittler" <nathan.mitt...@gmail.com>
Subject Re: Regarding feedback on OpenWire C++ client
Date Tue, 28 Mar 2006 11:28:45 GMT
On 3/28/06, David Fahlander <David.Fahlander@portwise.com> wrote:
>
> Hi Nathan,
>
> > > 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.
>
> Regarding objects that the user provides, we still get benefits by using
> smart pointers for them. It is not always the creator that is
> responsible of deletion. Especially in a threaded application it can be
> important to not end-up with ghost pointers. A thread in our client
> library that holds a pointer to a listener, must be able to rely on that
> the instance is still alive as long as it holds the SP to it. If the
> user would provide an ordinary pointer, the user must never delete it as
> long as there might be threads in our library using it. What about
> "static" objects that should last forever in the client's lifetime then?
> Still, we want to be able to take down the client in a controlled way.
> Doing that without smart pointers, may create a lot of problems in our
> experience.
>
> The "burden" for the user would only lie in the classes that implement
> our interfaces. If our API requires a p<IMessageListener>, the user must
> hold his message listener using a p<MyMessageListener> allocated with
> new MyMessageLister (...);. The user does not have to let this affect
> his design. If the user wants, this instance can lie as a member within
> another class that is neither derived from our interface nor maintained
> as a smart pointer.


You get no benefits at all by adding smart pointers to the interface.  The
user will delete their objects whenever they wish, regardless of what the
openwire library does ... and if it does so, the openwire library will
explode as soon as it tries to reference it.  That's the thing - this is C++
and there is no mechanism to prevent someone from coding badly, and adding
smart pointers to the API isn't going to change that.  The user is
responsible for both adding and removing referenes to its objects.  And it's
not the job of the openwire lib to worry about memory management in the
user-domain - it should only be concerned with its own memory management.

In addition, smart pointers are assuming that the object is allocated on the
heap, as it will do a "delete" when the last reference is removed.  This is
wrong, because the user should be able allocate its objects in whatever way
makes sense for the application.

Also, adding smart pointer arguments to all the methods on the api makes it
complicated and ugly.  If the openwire library wants to use smart pointers
internally, that's fine, but it shouldn't impose the use of smart pointers
on the user.  It's our job to make the user's experience a good one so they
continue to use ActiveMQ in their applications.


> > 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.
>
> The options we have regarding passing and holding strings are the
> following:
>
> Class Xxx {
>   p<string> name;
> public:
>   void setName (p<string> name) {
>     this->name = name;
>   }
>   p<string> getName () {
>     return this->name;
>   }
>   p<string> createName() {
>     return new string ("name: " + *name);
>   }
> };
>
> In the above example, strings are never copied, just the ptr value. The
> downside is that the caller needs to pass their strings as p<string>.
> This version adds the least overhead.


It's all about the user and helping them come up to speed and use the api as
quickly and painlessly as possible.  I understand that in these cases you're
passing a pointer and not copying, but it complicates the user interface
when the person just wants to pass in a string.  They shouldn't have to
create a string on the heap an then wrap it in a smart pointer to just call
a function.

---
>
> Class Xxx {
>   string name;
>   void setName (const string& name) {
>     this->name = name;
>   }
>   const string& getName () {
>     return this->name;
>   }
>   string createName() {
>     return string ("name: " + name);
>   }
> };
>
> In this example the string is always copied in the setters but not in
> getters.
>
> ---
>
> Class Xxx {
>   const char* name;
>   void setName (const char* name) {
>     this->name = name;
>   }
>   const char* getName () {
>     return this->name;
>   }
>   const char* () {
>     const char* val = new char[256];
>     strcpy (val, "name: ");
>     strcat (val, name);
>     return val;
>   }
> };
>
> This example is unacceptable since ownership is undefined.


I think you were missing my point.   I would change the example to this:

 Class Xxx {
  std::string name;
  void setName (const char* name) {
    this->name = name;
  }
  const char* getName () const{
    return this->name.c_str();
  }
};

OR ...

Class Xxx {
  std::string name;
  void setName (const std::string& name) {
    this->name = name;
  }
  const std::string& getName () const {
    return this->name;
  }
};

This way there is one copy done in the setter, but this bridges the gap
between the user domain and our lib.  Once that's been done, the user can
access the variable via the getter all day without copying.  This is a
standard way of handling strings in an application.

We are open to use any of the first two examples.
>
> Regards,
> David & Mats
>

Regards,
Nate

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message