Best practices question

Hello all,
   I have a best practices question.  I need to pass a pointer back from a function that creates the object.  Right now I'm writing code like this:

   CMyObject *GetObject()
   {
         static CMyObject *Obj = new CMyObject();
          .....
          return Obj;
    }

Is this the best way to pass back an object pointer?

Thanks.
LVL 1
edcAsked:
Who is Participating?
 
n_fortynineCommented:
Have you considered using smart pointers? The BOOST libraries have them and they're free for downloading. (www.boost.org)
0
 
elukaCommented:
Firstly why can't you store a pointer to the object inside a class and then call getter like this
class SomeClass
{
public:
SomeClass() { obj = new CMyObject(); }
~SomeClass() { delete obj; obj = NULL; }
CMyObject* GetObject() { return obj; }

private:
CMyObject *obj;
}

Secondly: in your code you probably have a memory leak. Where will you delete memory that was allocated in this function?

If you will be more specific about purpose of the function I could say more.

Eugene
0
 
n_fortynineCommented:
>> Firstly why can't you store a pointer to the object inside a class
eluka, I don't think that's what edc is asking. And I disagree with this. Technically he CAN do that. It will throw away the entire concept of having a class but it's only bad programming.
>> I need to pass a pointer back from a function that creates the object.
edc your code will do just fine. There will be issues concerning *deleting* the CMyObject created, but I guess by doing this you're entrusting that to the caller of the function.
0
Cloud Class® Course: C++ 11 Fundamentals

This course will introduce you to C++ 11 and teach you about syntax fundamentals.

 
elukaCommented:
Well, the code was OK but I wander where can I use the code like this ? It's certanly not the C++ way to do it :) I may be wrong.
0
 
edcAuthor Commented:
The function is a find function where I have a list of MyObjects.  I want to find the object in a list and pass it back to the caller function.  Since I am passing it back and it is to be used outside of the find function I can't have the reference deleted once the app moves outside the scope of the find function.
0
 
n_fortynineCommented:
For a quick an example here's something written by Sandu Turcan (I Googled this):
http://www.codeproject.com/cpp/ismart.asp
0
 
burcarpatCommented:
the best practice would be to avoid the pointer all together.  if you have a list of MyObjects, then switch to a stl container and return an iterator.  if you need to create this object on the heap, then delegate the creation to another member function which is synced with a cleanup function.  ideally, this member function should create the object and put it in the container.  then, you can simply return an iterator again

this looks like a typically case where you actually don't need pointers at all.  i say, avoiding them is definitely an option you should opt for

-- ba
0
 
rstaveleyCommented:
The function you've written is called a "source". A "source" creates objects and a "sink" deletes them.

 CMyObject *GetObject()
 {
         CMyObject *Obj = new CMyObject(); /* No need for the static here */
          .....
          return Obj;
 }

There is nothing all that wrong with writing functions in this way, but it is better practice to return an auto_ptr to ensure that calling code behaves properly as a sink and remembers to deletes the object, when it goes out of scope.

 /* Here's an explicitly defined source. A source returns an auto_ptr */
 std::autoptr<CMyObject> GetObjectFromSource()
 {
         std::auto_ptr<CMyObject> ptr(new CMyObject());
          .....
          return ptr; /* Return an auto_ptr, effectively passing ownership of object to the calling code */
 }

 /* Here's the sink */
 int main()
 {
    ....
    { /* Here's the start of the scope */
        /* We are the sink, and here we get ownership of an instance of CMyObject */
        std::auto_ptr<CMyObject> p = GetObjectFromSource();

        p->DoSomething(); /* Use the auto_ptr just like you would use a normal pointer */

    } /* The destructor for CMyObject gets called when the auto_ptr goes out of scope */
    ....
 }
0
 
efnCommented:
I can think of three ways a function could put out a pointer to an object.

1.  It could return the pointer as the function's return value.

2.  It could take a pointer1 or reference to a pointer2 and set the pointer2 via the pointer1 or reference.

3.  It could set a global variable.

Using a global variable is definitely not a best practice, so we can knock that one out.

If the function has only one output, it should be function's return value, in my humble opinion, although it wouldn't be a great sin to use a pointer or reference to a pointer.  If the function has more than one output, considerations for selecting which one should be the return value include:

--the return value cannot be an array

--if one of the outputs is a status or outcome value, it should be the return value

--the return value should be chosen for the convenience of the programmer of the calling function

--consistency with other similar functions is valuable

Once you select the output that is the return value, either it will be the object pointer or it won't.  If it's not, the object pointer should be set through a reference or pointer to pointer argument.  Again, this is just my personal opinion.

As other experts have advised you, returning a pointer to an object may or may not be the best design, and there may be memory management issues, but I am guessing that you are just asking about getting a pointer to an object out of a function, not asking for a review of your design.  If you are actually asking for a review of your design, it would be helpful if you described it in more detail.

--efn
0
 
fsign21Commented:
>The function is a find function where I have a list of MyObjects.
>I want to find the object in a list and pass it back to the caller function.
>Since I am passing it back and it is to be used outside of the find function
>I can't have the reference deleted once the app moves outside the scope of the find function.

If the object, containing the list, does not goes outside the scope after the call to the find-function, it is save to return a reference to the found object.
Example:
class Container
{
list<MyObject> obj;
public:
MyObject& find(int key)
{
   list<MyObject>::iterator iter = find(obj.begin(), obj.end(), Compare(key));
   if(iter == obj.end()) {
       // not found
       throw ObjectNotFoundExeption();
   }
   return (*iter);
}
};

int main()
{
Container cont;
cont.loadValuesFromSource();

try {
MyObject& curr = cont.find(25);
//curr is a valid reference here
.....
}
catch (ObjectNotFoundExeption& err)
{ . . . }
}


If the object, containing the list, goes outside the scope after the call to the find-function, it is better to return a copy of the found object instead of the pointer, since you do not have any performance advantages by returning a pointer to this object.
If you create the object like you did:
CMyObject *Obj = new CMyObject(FoundObject)
You have to copy the object anyway, plus you have to care about memory management . . .

So, I would do it like this
MyObject find(int key)
{
   list<MyObject>::iterator iter = find(obj.begin(), obj.end(), Compare(key));
   if(iter == obj.end()) {
      // not found
     throw ObjectNotFoundExeption();
   }
   return (*iter);
}
0
 
tinchosCommented:
This question has been classified as abandoned.  I will make a recommendation to the moderators on its resolution in approximately one week.  I would appreciate any comments by the experts that would help me in making a recommendation.

It is assumed that any participant not responding to this request is no longer interested in its final deposition.

If the asker does not know how to close the question, the options are here:
http://www.experts-exchange.com/help.jsp#hs5

Tinchos
EE Cleanup Volunteer
0
 
tinchosCommented:
No comment has been added lately, so it's time to clean up this question.
I will leave the following recommendation for this question in the Cleanup topic area:

Split: n_fortynine {http:#9621242} & burcarpat {http:#9621755}

Please leave any comments here within the next four days.
PLEASE DO NOT ACCEPT THIS COMMENT AS AN ANSWER!

Tinchos
EE Cleanup Volunteer
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.