Can some one tell me whats worng with the bellow code(C++)?

Question

The bellow code works fine unless I don't define destructor for the class. But it produces wrong result when I define destructor . In the destructor part I freed up the memory that was created in constructor .I think its my job to do so . But in this case doing this make my program to run with undesired output. I am very frustrated with the program. Please help me out. I can not figure out any problem with the code. All things seem to okay for me. Here I have made an array of objects of type list. Then I assigned values to the objects individually with the help of constructor. But it seems that the values are not getting initialize appropriately .But I think had not made any mistake there to initialization process. Then when I print the object data with member function display it could not produce the desire result. Although it prints the int and float data member properly. it can not print the char* data member properly.

#include <iostream>
#include <cstring>
#include <iomanip>
using namespace std;

class list
{
private:
    char*  name;
    int code;
    double cost;
public:
    //constructor
    list()
    {

    }
    list(const char* name,int code,double cost)
    {
        int len=strlen(name);
        this->name=new char[len+1];
        strcpy(this->name,name);
        this->code=code;
        this->cost=cost;

    }
    //mem fun
    void display(void)
    {
        cout.precision(2);
        cout.setf(ios::fixed);
        cout << setw(20) << this->name << setw(10) << this->code << setw      (12)    << cost << endl;
    }
    //destructor
    ~list()
    {
        delete[] this->name;
    }
};

int main()
{
    list item[3];

    item[0]=list("Turbo C++",1001,250.95);
    item[1]=list("C Primer",905,95.70);
    item[2]=list("C++ Book",105,30);

    cout << setw(20) << "NAME" << setw(10) << "CODE" << setw(12) << "COST"   <<     endl;

    for(int i=0;i<3;i++)
        item[i].display();

    return 0;
}

I got `free(): double free detected in tcache 2 /srv/wrappers/cpp-clang: line 5: 31205 Aborted`. You call a `delete[]` for default constructed elements but you didn't call a `new` for them. Use std container like `std::string` and `std::array` or `std::vector`. — Thomas Sablik, Jul 24 '19 at 08:00
Question regarding *why isn't this code working?* do not have their place here on StackOverflow. Your code is poorly formatted and you haven't provided any errors or output. Also, since you're not programming C, why don't you use `std::string` for example? — Victor, Jul 24 '19 at 08:01
Possible duplicate of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — DeviatioN, Jul 24 '19 at 08:44

score 0 · Answer 1 · answered Jul 24 '19 at 08:01

0

You are copying all your list items because you have an array of objects (not not pointers) - since it is just a "shallow copy" you have two instances with the same name pointer hence the "double free" message.

There are a couple of approaches to fix it:

You could define an operator= that makes a duplicate string. That works but you still have unnecessary copies.
You could have an array of pointers in main (instead of instances).
You could have a set() method to set the values inside an existing instance (e.g. item[0].set("Turbo C++", 1001, 250.95); )

answered Jul 24 '19 at 08:01

John3136

28,809
4
51
69

Or provide a move assignment operator – DeviatioN Jul 24 '19 at 08:46
I don't get your answer. Where is shallow copy. Doesn't strcpy makes a deep copy? – Ayush Mahajan Jul 24 '19 at 08:54
@AyushMahajan `strcpy` does a deep copy but it won't be called in the `item[0]=list(...)` lines. It will be called once by the constructor but the the `=` is just doing a byte copy so now you have 2 objects with the same pointer - the one in the list and the anonymous one used as the RHS of the assignment. – John3136 Jul 24 '19 at 09:10

Can some one tell me whats worng with the bellow code(C++)?

1 Answers1