Populating a pointer array of abstract objects with derived objects

[tanatos.daniel]

I have an abstract class CArticle and two derived classes CBook and CMagazine. I also have a class CLibrary where i want to add articles. Its members are an int variable to count the articles and a double pointer of CArticle.

I have the following main function:

int main()
{
    CLibrary L1;
    CArticle *A1=new CBook(1000);
    CArticle *A2=new CBook(1001);
    CArticle *A3=new CMagazine(1002,3);
    CArticle *A4=new CMagazine(1003,6);
    CArticle *A5=new CMagazine(1004,8);

    L1.addArticle(A1);
    L1.addArticle(A2);
    L1.addArticle(A3);
    L1.print();
}

I can't figure out what type of parameter should my addArticle() function have in order to work for this main. I would like to let the compiler choose if the object passed is a CBook or a CMagazine. Is that possible?

[Unimportant]

addArticle( ... ) will be interpreted in the context of the pointer.
You'll want to cast it first,

// C++11
auto Magazine((CMagazine*)A3);
// C++3
CMagazine * Magazine((CMagazine*)A3);

Before you call the function itself

Magazine->doSometing(  );



// To be quite honest it doesn't really matter what the buffer pointer is
L1.addArticle((void*)Thing); // Some other coders may yell at you if this isn't well documented

[deceptikon]

I can't figure out what type of parameter should my addArticle() function have in order to work for this main. I would like to let the compiler choose if the object passed is a CBook or a CMagazine. Is that possible?

Perhaps I'm missing something, but if you want polymorphism, you should use polymorphism rather than trying something tricky and dangerous:

#include <iostream>

class CArticle {
public:
    virtual void print() const = 0;
};

class CBook: public CArticle {
    int _x;
public:
    CBook(int x) : _x(x) {}

    virtual void print() const override
    {
        std::cout << "CBook: " << _x << '\n';
    }
};

class CMagazine: public CArticle {
    int _x;
    int _y;
public:
    CMagazine(int x, int y) : _x(x), _y(y) {}

    virtual void print() const override
    {
        std::cout << "CMagazine: " << _x << ' ' << _y << '\n';
    }
};

class CLibrary {
    CArticle **_articles;
    int _size;
public:
    CLibrary() : _articles(new CArticle*[10]), _size(0) {}

    ~CLibrary()
    {
        delete[] _articles;
    }

    void addArticle(CArticle *article)
    {
        _articles[_size++] = article;
    }

    void print() const
    {
        for (int i = 0; i < _size; i++) {
            _articles[i]->print();
        }
    }
};

int main()
{
    CLibrary L1;
    CArticle *A1 = new CBook(1000);
    CArticle *A2 = new CBook(1001);
    CArticle *A3 = new CMagazine(1002, 3);
    CArticle *A4 = new CMagazine(1003, 6);
    CArticle *A5 = new CMagazine(1004, 8);

    L1.addArticle(A1);
    L1.addArticle(A2);
    L1.addArticle(A3);
    L1.addArticle(A4);
    L1.addArticle(A5);

    L1.print();
}

[rubberman]

I would add one change to the structures/classes/members shown by decepticon, and that is to use a std::vector<CArticle> for the _articles member in the CLibrary class. Also, it is STRONGLY recommended by standards to NOT use leading underscores for local/class/static variables as that is reserved for internal use by system libraries, etc. IE, use m_articles instead. Also, the std::vector class has the advantage of being able to automatically resize itself when you add more elements than it may have been configured for in the first place. In decepticon's example, you would have to resize it manually in your addArticle() method if the size of the array has reached its previous limit.

[deceptikon]

I would add one change to the structures/classes/members shown by decepticon, and that is to use a std::vector<CArticle> for the _articles member in the CLibrary class.

I would too, but given that the description of the actual classes suggests a dynamic array of pointers, I matched my example to that.

Also, it is STRONGLY recommended by standards to NOT use leading underscores for local/class/static variables as that is reserved for internal use by system libraries, etc.

Identifiers starting with two underscores or an underscore followed by an upper case letter are always reserved, period. An identifier starting with an underscore followed by a digit or a lower case letter is not reserved except in the global namespace. My data member names are perfectly legal and conforming.

However, with that said, it couldn't hurt to always avoid leading underscores as a general best practice. The only reason I do it is because I'm confident in my understanding of the rules and restrictions.

In decepticon's example, you would have to resize it manually in your addArticle() method if the size of the array has reached its previous limit.

Once again, my example wasn't intended to be a full solution. I assumed that the OP's classes were already otherwise feature complete since he didn't ask about anything except adding polymorphic items in the interface.

[Unimportant]

@deceptikon
Your solution is wrong.
In your solution, when he calls print() it will treat all of the pointers as articles no matter what they are.

[tanatos.daniel]

@deceptikon

I also tried this, using an overloaded assignement operator defined for each one of the classes CArticle,CBook,CMagazine, but it breaks somewhere.

void addArticle(CArticle *article)
{
    _articles[_size++] = article;
}

My guess is the overloaded operator redefined by me, having the following descriptions, is not doing what i would want because of the parameter passed by reference.

CArticle &CArticle::operator =(CArticle &A)
{
    if (this==&A)
        return *this;

    mIndex=A.mIndex;

    return *this;
}





CBook &CBook::operator =(CBook &A)
{


    if (this==&A)
            return *this;

        mIndex=A.mIndex;

        return *this;


}





CMagazine &CMagazine::operator =(CMagazine &A)
{
    if (this==&A)
        return *this;

    mIndex=A.mIndex;
    mMonth=A.mMonth;

    return *this;
}

I also tried to overload the assignement operator this way:

CArticle *CArticle::operator =(CArticle *A)
{
    ...
}

though i doubt it is correct..

The code breaks at void addArticle(CArticle *article) saying Access violating writing location. My CLibrary constructor looks like this:

CLibrary::CLibrary()
{
    mA=new CArticle*[MAX];  //#define MAX 10
    mSize=0;
}

[deceptikon]

@deceptikon
Your solution is wrong.
In your solution, when he calls print() it will treat all of the pointers as articles no matter what they are.

You might want to educate yourself a bit more before trying to correct people who actually know what they're talking about.

I also tried this, using an overloaded assignement operator defined for each one of the classes CArticle,CBook,CMagazine, but it breaks somewhere.

The overloaded assignment operator is irrelevant here because you're working with pointers at the container level. Please post a small but complete program that exhibits your problem and I'll see if I can locate the error.

[Unimportant]

@deceptikon

You must be high, at run-time the container object won't know which object is a derived type (versus being a base type) unless he defines the type within the class and casts the base class pointer to the derived class pointer.

@tanatos

In your code, everything will be treated as article within Library.
That means if you have,
CArticle *A1 = new CBook(1000);

and you call,
A1->print( );
you'll see the output of:
virtual void print() const = 0;

[tanatos.daniel]

You're right. The operator is irrelevant.

This would be a short version of my code:

#include <iostream>
#include <conio.h>

using namespace std;

#define MAX 10

class CArticle
{
    public:
        int mIndex;

        CArticle::CArticle()
        {
            mIndex=0;
        }

        CArticle::CArticle(int c)
        {
            mIndex=c;
        }

        CArticle::CArticle(CArticle &A)
        {
            mIndex=A.mIndex;
        }

        virtual void print() = 0;

        CArticle::~CArticle() {}


};

class CBook : public CArticle
{
    public:
        char *mAuthor;

        CBook::CBook():CArticle()
        {
            mAuthor=NULL;
        }

        CBook::CBook(int c,char *a):CArticle(c)
        {
            mAuthor=strdup(a);
        }

        CBook::CBook(CBook &C):CArticle(C.mIndex)
        {
            mAuthor=strdup(C.mAuthor);
        }

        virtual void CBook::print()
        {
            cout<<"Author: "<<mAuthor<<endl
                <<"Index: "<<mIndex<<endl;
        }

        CBook::~CBook() {}
};

class CMagazine : public CArticle
{
    public:
        int mMonth;

        CMagazine::CMagazine():CArticle()
        {
            mMonth=0;
        }

        CMagazine::CMagazine(int c,int l):CArticle(c)
        {
            mMonth=l;
        }

        CMagazine::CMagazine(CMagazine &CD):CArticle(CD.mIndex)
        {
            mMonth=CD.mMonth;
        }

        virtual void CMagazine::print()
        {
            cout<<"Month: "<<mMonth<<endl
                <<"Number: "<<mIndex<<endl;
        }

        CMagazine::~CMagazine() {}
};

class CLibrary
{
    public:
        CArticle **mA;
        char *mName;
        int mNr;

        CLibrary::CLibrary()
        {
            mA=NULL;
            mName=NULL;
            mNr=0;
        }

        CLibrary::CLibrary(char *n)
        {
            mA=new CArticle*[MAX];
            mName=strdup(n);
            mNr=0;
        }

        CLibrary::CLibrary(CLibrary &B)
        {
            mA=new CArticle*[MAX];
            mA=B.mA;
            mName=strdup(B.mName);
            mNr=B.mNr;
        }

        void CLibrary::addArticle(CArticle *A)
        {
            mA[mNr]=A;      //here it breaks
            mNr++;
        }

        void CLibrary::print()
        {
            for (int i=0; i<mNr; i++)
                mA[i]->print();
        }

        CLibrary::~CLibrary()   {}
};

int main()
{
    CLibrary B1;
    CArticle *C1=new CBook(1000,"John");
    CArticle *C2=new CBook(1001,"James");
    CArticle *C3=new CMagazine(1002,3);

    B1.addArticle(C1);
    B1.addArticle(C2);
    B1.addArticle(C3);
    B1.print();

    getch();
}

[deceptikon]

You must be high, at run-time the container object won't know which object is a derived type (versus being a base type) unless he defines the type within the class and casts the base class pointer to the derived class pointer.

The OP is storing pointers to a base class, not objects of the base class. Read up on how polymorphism and dynamic binding work in C++ and then look at my code again, because you're clearly confused.

[Unimportant]

@deceptikon

You may want to read an intro level tutorial to CPP
http://www.learncpp.com/cpp-tutorial/121-pointers-and-references-to-the-base-class-of-derived-objects/

Objects don't actually recreate their function data (it's not included in structure size) when creating multiple objects.
They refer to a static address table of functions, if the compiler thinks the pointer is a base class it will call the base class function.

[deceptikon]

This would be a short version of my code:

You created CLibrary using the default constructor. mA is a null pointer.

@deceptikon
You may want to read an intro level tutorial to CPP
http://www.learncpp.com/cpp-tutorial/121-pointers-and-references-to-the-base-class-of-derived-objects/

Good. Now go to the next chapter and read about virtual functions, genius. I'd also appreciate if you reversed your negative rep on me when you eventually realize I'm right.

[Unimportant]

Ok, the code you included is correct.

[tanatos.daniel]

Forgot to check that. Thanks a lot!

The problem was way simpler than I imagined..

[mike_2000_17]

There are also a number of additional serious problems with your code.

1) Your copy-constructor is wrong:

    CLibrary::CLibrary(CLibrary &B)
    {
        mA = new CArticle*[MAX];
        mA = B.mA;    // <-- Problem! Assign the pointer 'B.mA' to the pointer 'this->mA'
                      //  the memory you just allocated (with new) is leaked, and both the 
                      //  original object 'B' and this object will refer to the same memory.
                      //  This is called a shallow copy, what you need is a deep copy.
                      //  Such as:
        // std::copy(B.mA, B.mA + MAX, mA);   // copy data in [B.mA, B.mA+MAX) to [mA, mA+MAX).
        mName = strdup(B.mName);
        mNr = B.mNr;
    }

2) The destructor of a polymorphic base-class should always be declared as virtual to make sure that the correct derived-class destructor will be called when you delete the base-class object (via a pointer to it). So, declare the base-class destructor virtual as so:

class CArticle
{
    public:
        int mIndex;

        CArticle(int c = 0) {  // note: easier to have default-value instead of two constructors.
            mIndex=c;
        }

        // note: copy-constructor is not needed here (the default version is correct).

        virtual void print() = 0;

        virtual ~CArticle() {};   // virtual destructor.
};

3) The destructor of the CLibrary class is also wrong because you don't delete any of the objects that your library has ownership of, nor do you actually delete the memory you use to store that array of pointers. So, you'd need something like this:

    CLibrary::~CLibrary() {
        for (int i=0; i<mNr; i++)
            delete mA[i];  // delete individual objects.
        delete[] mA;  // delete the array.
    };

Now, I hope you can see that there is a problem here. What if the individual objects where not allocated with new, or what if individual objects were already deleted from elsewhere (e.g., from the main() function). These are problematic issues. You could say "I require all the pointers added to the library as being allocated with new and not being deleted by anywhere else after being given to this library". But that won't work if you allow libraries to be copied, and so on. So, you might instead require that objects be deleted outside of the library, as in, having a destructor as so:

    CLibrary::~CLibrary() {
        delete[] mA;  // just delete the array.
    };

which would lead to the main function being something like this:

int main()
{
    CArticle *C1=new CBook(1000,"John");
    CArticle *C2=new CBook(1001,"James");
    CArticle *C3=new CMagazine(1002,3);

    { // scope in which the library is created and used.
        CLibrary B1;
        B1.addArticle(C1);
        B1.addArticle(C2);
        B1.addArticle(C3);
        B1.print();
    }; // library is destroyed here. Articles are no longer needed.

    delete C1;   // delete the objects 'outside' of the library, 
    delete C2;   // and only when you don't need to library anymore.
    delete C3;

    getch();
}

In all cases, this is a mess to deal with, and you should consider using smart-pointers in the future, especially std::shared_ptr which just wraps a pointer with a reference counter that will automatically destroy the object when there are no more references (pointers) to that object.

Also, in terms of having to deal with a raw array of pointers within your library class, that is also far less than ideal. To write such a class correctly, you must write a RAII class (or resource-holding class), as explained here. Or, far simpler, just use a C++ container, in particular for this case, use std::vector, as so:

class CLibrary
{
    private:
        std::vector<CArticle*> mA;
        std::string mName;

    public:

        CLibrary::CLibrary(const std::string& aName = "") {
            mName = aName;
        }

        void CLibrary::addArticle(CArticle *A)
        {
            mA.push_back(A);  // add an article to the vector of articles.
        }

        void CLibrary::print()
        {
            for (int i = 0; i < mA.size(); i++)
                mA[i]->print();
        }
};

As you see, by using std::vector for the array of articles and by using std::string for the name of the library, you eliminate the need to have a copy-constructor, a copy-assignment operator and a destructor because both of these classes are standard RAII classes that manage their own resources (memory in this case) safely in terms of copying and destroying, so you don't have to worry about it. This is by far preferred to using C-style things like char* for strings or raw pointers for dynamically-allocated arrays.

[tanatos.daniel]

We are not allowed to use STL, so i don't know very much of them yet. Hopy i'll study them one day. From what i've read so far, it makes life of a c++ programmer simpler.

Thank you, mike, for all advices. They were very useful. I'll post more of my questions here, for guys like you to guide me. Didn't hope for help when i posted this one.