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?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

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
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.
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.
OWASP: Forgery and Phishing

Learn the techniques to avoid forgery and phishing attacks and the types of attacks an application or network may face.

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.
n_fortynineCommented:
Have you considered using smart pointers? The BOOST libraries have them and they're free for downloading. (www.boost.org)

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
n_fortynineCommented:
For a quick an example here's something written by Sandu Turcan (I Googled this):
http://www.codeproject.com/cpp/ismart.asp
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
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 */
    ....
 }
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
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);
}
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
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
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
C++

From novice to tech pro — start learning today.