C++: Inheritance, assigning

Question

I'm learning C++, currently I'm at inheritance of classes and dynamic memory allocation for objects. I'm doing some exercises, and right now I'm stuck on some strange crash of my application.

The problem lies in assigning derived class to new object. Everything looks fine, until destructors are starting to work, I analyzed everything and I cannot find mistake. Code should be pretty simple, class cd stores simple data and "classic" is adding one field.

(The main problem is that app is crashing at the end)

This is source code I hope someone will be able to help me

#ifndef CLASSIC_H_
#define CLASSIC_H_

class Cd
{
private:
    char* performers;
    char* label;
    int selection;
    double playtime;
public:
    Cd(char* s1, char* s2, int n, double x);
    Cd(const Cd& d);
    Cd();
    virtual ~Cd();
    virtual void Report() const;
    Cd& operator=(const Cd& d);
};

class Classic : public Cd
{
private:
    char* maintrack;
public:
    Classic(char* mt, char* s1, char* s2, int n, double x);
    Classic();
    Classic(const Classic& c);
    Classic(char* mt, const Cd& d);
    virtual void Report() const;
    virtual ~Classic();
    Classic& operator=(const Classic& c);
};

#endif



using std::strcpy;
Cd::Cd(char* s1, char* s2, int n, double x)
{

    performers = new char[strlen(s1) + 1];
    strcpy(performers, s1);

    label = new char[strlen(s2) + 1];
    strcpy(label, s2);

    selection = n;
    playtime = x;
}

Cd::Cd(const Cd& d)
{
    performers = new char[strlen(d.performers) + 1];
    strcpy(performers, d.performers);

    label = new char[strlen(d.label) + 1];
    strcpy(label, d.label);

    selection = d.selection;
    playtime = d.playtime;
}    

Cd::~Cd()
{

    delete [] performers;
    delete [] label;
}

Cd::Cd()
{
    performers = new char[1];
    performers[0] = '\0';

    label = new char[1];
    label[0] = '\0';

    selection = 0;
    playtime = 0;
}


Cd& Cd::operator=(const Cd& d)
{
    if (this == &d)
        return *this;

    delete [] performers;
    delete [] label;

    performers = new char[strlen(d.performers) + 1];
    strcpy(performers, d.performers);

    label = new char[strlen(d.label) + 1];
    strcpy(label, d.label);

    selection = d.selection;
    playtime = d.playtime;
    return *this;
}


void Cd::Report() const
{
    using namespace std;
    cout << performers << endl;
    cout << label << endl;
    cout << selection << endl;
    cout << playtime << endl;
}


Classic::Classic(char* mt, char* s1, char* s2, int n, double x)
    : Cd(s1, s2, n, x)
{
    maintrack = new char[strlen(mt) + 1];
    strcpy(maintrack, mt);
}

Classic::Classic() : Cd()
{
    maintrack = new char[1];
    maintrack[0] = '\0';
}

Classic::Classic(const Classic& c) : Cd(c)
{
    maintrack = new char[strlen(c.maintrack) + 1];
    strcpy(maintrack, c.maintrack);
}

Classic::Classic(char* mt, const Cd& d) : Cd(d)
{
    maintrack = new char[strlen(mt) + 1];
    strcpy(maintrack, mt);
}

void Classic::Report() const
{
    Cd::Report();
    std::cout << maintrack << std::endl;
}

Classic::~Classic()
{
    delete [] maintrack;
}

Classic& Classic::operator=(const Classic& c)
{
    if (this == &c)
        return *this;

    Cd::operator=(c);
    delete [] maintrack;

    maintrack = new char[strlen(c.maintrack)];
    strcpy(maintrack, c.maintrack);

    return *this;
}







#include <iostream>
#include "classic.h"
#include <cstdlib>
using namespace std;

void Bravo(const Cd& disk);

int main()
{
    Cd c1("Beatles", "Capitol", 14, 35.5);
    Classic c2 = Classic("Sonata fortepianowa B-dur, Fantazja C-moll",
                         "Alfred Brendel", "Philips", 2, 57.17);


    Classic copy;
    copy = c2;

    copy.Report();


    system("pause");


    return 0;
}

void Bravo(const Cd& disk)
{
    disk.Report();
}

Answer 1

The problem is in your copy constructor for Classic. However, this just highlights the perils of doing your own string manipulation. I'd suggest rewriting this entire exercise with no char* and using std::string instead.

Classic& Classic::operator=(const Classic& c)
{
    if (this == &c)
        return *this;

    Cd::operator=(c);
    delete [] maintrack;

        // you need strlen(c.maintrack) + 1
    maintrack = new char[strlen(c.maintrack)];
    strcpy(maintrack, c.maintrack);

    return *this;
}

Answer 2

Possible memory corruption in Classic& Classic::operator=(const Classic& c) :

maintrack = new char[strlen(c.maintrack)];
strcpy(maintrack, c.maintrack);

replace with:

maintrack = new char[strlen(c.maintrack) + 1];
strcpy(maintrack, c.maintrack);

That said, why exactly aren't you just using the string class that's part of the standard library? It would save you this and many other headaches. It does all of the memory management, null termination, etc. all automatically.

string maintrack;
char * derp = "derp";
maintrack = derp; // valid
maintrack = "derp"; // valid
maintrack = another_string; // valid