C++ How can I construct an object inside a class function, do some operations and return the value?

StackOverflow https://stackoverflow.com/questions/22823925

  •  26-06-2023
  •  | 
  •  

Question

I am not going to past the whole source because it is 1000+ rows, but I have specially constructed a similar case about the matter I am interested in. Pay attention to this source code:

#include <iostream>
using namespace std;

class Person
{
public:
    Person();
    Person(char*);
    ~Person();
    Person& operator=(const Person&);
    friend Person& example(const Person&);
    void print() const;
private:
    char* name;
};

Person::Person()
{
    name = new char[12];
    name = "Temp";
}

Person::~Person()
{
     delete[] name;
}

Person::Person(char* _name)
{
    name = new char[strlen(_name)+1];
    strcpy_s(name,strlen(_name)+1,_name);
}

Person& example()
{
    char* TestName = new char[11];
    TestName = "ShouldStay";
    Person B(TestName);
    return B;
}

void Person::print() const
{
    cout << name;
}

int main()
{
    example();
    return 0;
}

In this case the example() function will return:

  • example returned {name=0x007cad88 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþ... } Person &

So obviously the destructor is called on the return command and delete the memory in the heap (so I am not able to do anything further with the pointer - it is pointing to already freed memory - no data).

My question is - How to avoid such behavior? What is the most elegant way to avoid such issue?

Thank you in advance!

Was it helpful?

Solution

  1. Use string rather than char[] to avoid having to use new.
  2. Return Person rather than Person&, as locally-scoped classes are destroyed after they leave scope. Although this will cause a copy to happen depending on compiler settings. This depends of course upon providing a proper copy constructor.
  3. To guarantee avoiding a copy, change the signature of example to:

    void example(Person& person)
    

    And fill in the fields of the inputted person inside the function. The scope of that Person will be bound to the calling scope (or wherever else you constructed it). This method has drawbacks though such as you cannot chain the results together.

OTHER TIPS

Your code contains many logical errors:

Person::Person()
{
    name = new char[12];
    name = "Temp";
}

In the above function you allocate a char array of 12 elements, then you simply forget about it and instead make name pointing to a string literal.

Person::~Person()
{
     delete[] name;
}

whoopps. In case Person was build from a default constructor this would delete a string literal. A no-no in C++.

Person::Person(char* _name)
{
    name = new char[strlen(_name)+1];
    strcpy_s(name,strlen(_name)+1,_name);
}

Not 100% sure what strcpy_s is, but the code in this case allocates an array and seems to copy the string into the array. This seems ok (but just strcpy(name, _name); would have been better for many reasons).

Person& example()
{
    char* TestName = new char[11];
    TestName = "ShouldStay";
    Person B(TestName);
    return B;
}

This code is seriously broken. First of all it's returning by reference a temporary object. A Very Very Bad Idea. It's also allocating an array, and once again just forgetting about it and using a string literal instead.

The most elegant way (actually the ONLY way in my opinion) to get your code working is to first understand how the basics of C++ work. You should start first by reading a good C++ book from cover to cover, and only then you should start coding in C++.

Your 1000 lines of source code are most probably just rubbish. I'm not saying you're dumb, just that you don't know the basics of C++. Take care of them first by reading, not experimenting with a compiler.

You cannot learn C++ by experimenting for two reasons:

  • It's a complicate and sometimes even just downright illogical language because of its history. Guessing is almost always a bad move. No matter how smart you are there's no way you can guess correctly what a committee decided.

  • When you make a mistake there are no runtime error angels to tell you so. Quite often it happens that apparently the program works anyway... until it's run by your teacher, boss or spouse. Guessing C++ rules by writing code and observing what happens is nonsense.

Person& example()
{
    char* TestName = new char[11];
    TestName = "ShouldStay";
    Person B(TestName);
    return B;
}

The above creates an instance of Person on the stack, scoped to the function. So when the function returns, the destructor for Person is called and a reference to the destroyed object (on the stack) is returned.

In addition you should either consider returning a copy of Person, or you need to use the new operator to create an instance of person on the heap and return a pointer to that.

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