Salvatore Sorbello
Salvatore Sorbello

Reputation: 606

const object& return value from a method, best practice

My C++ knowledge are minimal, for this i'm asking this:

I have a pointer to an object of type Ndb and i do this:

Ndb* myObj=new Ndb();
NdbError err=myObj->getNdbError();
//Do some work;
//Finish the job
delete(myObj);

The method's signature for getNdbError is:

const NdbError& getNdbError() const

According to the signature, who must take care of free the memory for the NdbError? the const before NdbError& means that i can only read the result and i should not to anything else? I don't wont to leave some memory area allocated but not referenced.

EDIT: According to the ansers its better to show more of this use case. I have a wrapper beetween a C++ object and C methods:

void* WINAPI new_Ndb(void* cluster_connection,const char* catalogname,const char* schemaName)
{

    Ndb_cluster_connection* co=(Ndb_cluster_connection*)cluster_connection;
    Ndb* tmpN=new Ndb(co,catalogname,schemaName);
    return (void*)tmpN;
}

void WINAPI Ndb_dispose(void* obj)
{
    delete (Ndb*)obj;
    obj=0;
}

const ndberror_struct WINAPI Ndb_getNdbError(void* obj)
{
    Ndb* tmp=(Ndb*)obj;

    NdbError res=tmp->getNdbError();    
    ndberror_struct tmpRes;
    tmpRes=(ndberror_struct)res;
    return tmpRes;
}

NdbError define an operator overloading:

  operator ndberror_struct() const {
    ndberror_struct ndberror;
    ndberror.status = (ndberror_status_enum) status;
    ndberror.classification = (ndberror_classification_enum) classification;
    ndberror.code = code;
    ndberror.mysql_code = mysql_code;
    ndberror.message = message;
    ndberror.details = details;
    return ndberror;
  }

What will appen when i make a call to

const ndberror_struct WINAPI Ndb_getNdbError(void* obj)?

The Ndberror will go out of scope but i keep the data because are copied? message and details are char* so they will be freed when the return value ndb_error_struct will go out of scope?

Upvotes: 0

Views: 152

Answers (4)

kenba
kenba

Reputation: 4539

The function const NdbError& getNdbError() const; returns a constant reference to an NdbError.

Your NdbError err copy will automatically be deleted at the end of the function.

However rather than making a copy it should be a constant reference, i.e. change:

 NdbError err=myObj->getNdbError();

to

 const NdbError& err=myObj->getNdbError();

then it's clear that myObj takes care of the memory for the NdbError.

Upvotes: 1

John Dibling
John Dibling

Reputation: 101456

The getNdbError() returns a const reference to a NdbError object. You do not delete references. You only delete something you newed.

You assign the return of getNdbError to an NdbError object with automatic lifetime, as opposed to dynamic lifetime:

NdbError err=myObj->getNdbError();

err here is an automatic variable. It will be destroyed automatically when it "falls out of scope."

It's interesting to note that, if the code you provided is the same as it is in your project, err will be destroyed after myObj, which you delete properly. If there is a link between err and myObj which will cause undefined behavior or some other bad stuff in err's destructor since myObj no longer exists, you may have to revisit your design.

I would strongly recommend either not using dynamic allocation at all, or if you must then at least wrap it in an RAII construct like a smart pointer so that allocation and deallocation can be handled in a more structured manner.

Upvotes: 1

Mike Seymour
Mike Seymour

Reputation: 254431

The function returns a reference to an object that's managed by myObj; perhaps one of its members. You don't need to do anything to release that object; myObj should take care of it when it is destroyed.

You are using the returned reference to initialise an automatic variable err to be a copy of it. Like all automatic variables, it will be destroyed automatically when it goes out of scope. You don't need to do anything to release that object either.

However, you should probably make myObj an automatic variable too, unless you have a good reason for dynamic allocation. Your code will have a memory leak if anything throws an exception between new and delete.

Upvotes: 1

Kiril Kirov
Kiril Kirov

Reputation: 38153

It most probably returns a reference to a data member, you don't need to delete it.

err will be a copy of the returned value from getNdbError, but as it's a stack variable, it will be destroyed at the end of the scope - nothing to worry about.

Upvotes: 2

Related Questions