Question

I was trying to brush up my c++ skills. I got 2 functions:

concat_HeapVal() returns the output heap variable by value
concat_HeapRef() returns the output heap variable by reference

When main() runs it will be on stack,s1 and s2 will be on stack, I pass the value by ref only and in each of the below functions, I create a variable on heap and concat them.

When concat_HeapVal() is called it returns me the correct output.
When concat_HeapRef() is called it returns me some memory address (wrong output). Why?

I use new operator in both the functions. Hence it allocates on heap.
So when I return by reference, heap will still be VALID even when my main() stack memory goes out of scope.

So it's left to OS to cleanup the memory. Right?

string& concat_HeapRef(const string& s1, const string& s2)
{

    string *temp = new string();
    temp->append(s1);

    temp->append(s2);
    return *temp;
}


string* concat_HeapVal(const string& s1, const string& s2)
{

    string *temp = new string();
    temp->append(s1);

    temp->append(s2);
    return temp;

}

int main()

{

    string s1,s2;
    string heapOPRef;
    string *heapOPVal;
    cout<<"String Conact Experimentations\n";
    cout<<"Enter s-1 : ";
    cin>>s1;
    cout<<"Enter s-2 : ";

    cin>>s2;

    heapOPRef = concat_HeapRef(s1,s2);
    heapOPVal = concat_HeapVal(s1,s2);

    cout<<heapOPRef<<"  "<<heapOPVal<<" "<<endl;
        return -9;
}
Was it helpful?

Solution

Both your functions are legal and fine, apart from that you're later leaking the free store objects from both functions.

The first function returns a reference to the value on the free store, which is copied into your local variable in main. The second function returns a pointer to the instance on the free store, which is stored into a pointer variable in main.

Your problem is in that you do not print the second string, you print the address of it, thanks to that there's an operator<< that prints the address a pointer contains as a number.

If you intend to access the contents of the second variable, you should dereference it when using it.

OTHER TIPS

When you return by reference, you signify (among other things) that the caller does not become owner of the data and is not responsible for freeing it.

You've allocated new block and the only remaining reference to it is the one you returned. So it cannot be by reference.

But it does not apply to all heap-allocated blocks. If you have heap-allocated block to which you hold pointer in member variable to be released later e.g. in destructor, you may and often will return it by reference.


But really, pointer to std::string is a code smell. std::string should most of the time should be returned by value. That is as string, string * is by pointer, not by value.

So it's left to OS to cleanup the memory. Right?

Right.

You leak the memory.

Your OS will have to reclaim the memory itself when your process ends, or it'll remain allocated until power down.

Please don't do this.

Stop using new immediately: just create a friendly new std::string object and return it. Let the compiler's optimisations and/or move semantics sort everything out.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top