hello,
i want to learn if there's memory leak problem in my code...Let me explain:
I have a Customer class,that class also have a destructor function.
I have a static function in that class which returns n Customer objects.

static Customer* createCustomers(int n){
//...
}
int main(int argc, char** argv) {
    int n=10;
    Customer* customers = Customer::createCustomers(n);
    for (int i=0;i<n;i++){
        customers[i].display();
    }
//delete customers;
}

These n Customer objects which are created by Customer::createCustomers function can be destructed by the destructor function successfully, but when I try to delete the customers pointer, "double freed" error occurs.because the customer objects are being tried to deleted again. if i don't delete the customers this will cause memory leak or not?

here's my whole code if needed..:

#include <cstdlib>
#include <cstdio>
#include <iostream>
using namespace std;
#define MAXBUF 80

class Customer{
private:
    int id;
    char *name, *surname, *address;
public:
    Customer(){
        this->id = 0;
        this->name=new char[MAXBUF];
        this->surname=new char[MAXBUF];
        this->address=new char[MAXBUF];
    }
    void set_id(int id_){this->id = id_;}
    void set_name(char* name_){strcpy(this->name, name_);}
    void set_surname(char* surname_){strcpy(this->surname, surname_);}
    void set_addres(char* address_){strcpy(this->address, address_);}
    int     get_id(){return this->id;}
    char* get_name(){return this->name;}
    char* get_surname(){return this->surname;}
    char* get_address(){return this->address;}
    void display(){
        cout<<"********Customer Info*********\n";
        cout<<"ID........:"<<this->get_id()<<endl;
        cout<<"Name......:"<<this->get_name()<<endl;
        cout<<"Surname...:"<<this->get_surname()<<endl;
        cout<<"Address...:"<<this->get_address()<<endl;
    }
    ~Customer(){
        delete name, surname, address;
        cout<<"Customer removed from the memory...\n";
    }
    static Customer* createCustomers(int n){
        Customer* customers=new Customer[n];
        char* id_str = new char[MAXBUF];
        char* namestr = new char[MAXBUF];
        char* surnamestr = new char[MAXBUF];
        char* addressstr = new char[MAXBUF];
        for (int i=0;i<n;i++){
            Customer c;
            strcpy(namestr, "name");
            strcpy(surnamestr, "surname");
            strcpy(addressstr, "address");
            sprintf(id_str, "%d", i);
            strcat(namestr, id_str);
            strcat(surnamestr, id_str);
            strcat(addressstr, id_str);
            c.set_id(i);
            c.set_name(namestr);
            c.set_surname(surnamestr);
            c.set_addres(addressstr);
            customers[i]=c;
        }
        delete namestr, surnamestr, addressstr;
        delete id_str;
        return customers;
    }
};

int main(int argc, char** argv) {
    int n=10;
    Customer* customers = Customer::createCustomers(n);
    for (int i=0;i<n;i++){
        customers[i].display();
    }
    //delete customers;
}

Recommended Answers

All 3 Replies

It leaks like a sieve.

> delete namestr, surnamestr, addressstr;
First of all, since you used [ ] in the new, you MUST use [ ] in the delete.
Second, the comma operator doesn't mean "do this to all these things", it means "do it to the last thing".
So all you've got at the moment is
delete addressstr;
whereas what you want is
delete [ ] namestr;
delete [ ] surnamestr;
delete [ ] addressstr;

Plus, the whole static create customers thing just seems wrong to me.

Plus, the whole static create customers thing just seems wrong to me.

I agree -- it would be better to allocate an array of Customer objects than to have one object allocate an array of data within that instance

int main(int argc, char** argv) {
    int n=10;
    Customer* customers = new Customer[n];
    for (int i=0;i<n;i++){
        customers[i].display();
    }
delete[] customers;
}

The class should also use std::string instead of char* because (1) its c++ instead of c, and (2) your program doesn't have to worry about memory allocation/deletion.

class Customer{
private:
    int id;
    std::string name;
    std::string surname;
    std::string address;
public:
    Customer(){
        this->id = 0;
    }
    void set_id(int id_){this->id = id_;}
    void set_name(std::string name_){this->name = name;}
    void set_surname(std::string surname_){this->surname = surname;}
    void set_addres(std::string address_){this->address = address;}
    int     get_id(){return this->id;}
    std::string get_name(){return this->name;}
    std::string get_surname(){return this->surname;}
    std::string get_address(){return this->address;}
    void display(){
        cout<<"********Customer Info*********\n";
        cout<<"ID........:"<<this->get_id()<<endl;
        cout<<"Name......:"<<this->get_name()<<endl;
        cout<<"Surname...:"<<this->get_surname()<<endl;
        cout<<"Address...:"<<this->get_address()<<endl;
    }
    ~Customer(){
      // nothing to do
    }
    // delete the static method because its not needed.
};

Looking at that static method some more, I think what you really want is a vector of Customer objects.

int main()
{
     vector<Customer> customers;

       for (int i=0;i<n;i++){
            Customer c;
            stringstream s;
            s << i;     
            c.set_id(i);
            c.set_name("name" + s.str());
            c.set_surname("surname" + s.str());
            c.set_addres("address" + s.str());
            customers.push_back(c);
        }

   ...
   ...
   vector<Customer>::iterator it;
   for(it = customers.begin(); it != customers.end(); it++)
         *it->display();
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.