Problem przy dziedziczeniu z list<A*>

Witam,

Po dość zawiłym tytule wyjaśnię o co chodzi. Zadanko mam takie: Klasy B,C,D dziedziczą po A. A list dziedziczy po list<A*> i może przechowywać A, B, C, D. do Alist napisz konstruktor, destruktor, konstruktor kopiujący, operator =, operator +. Problem w moim kodzie jest w operatorze + i coś nie wiem chbya jeszcze o dziedziczeniu bo nie mogę go zdemaskować. Oto kod:

 
#include <cstdlib>
#include <iostream>
#include <list>

using namespace std;

class A
{
      public:
             virtual ~A() {}
};

class B : public A
{
      public:
             B() {}
             ~B() {}
             void wypisz()
             {
                  cout << "B" << endl;
             }
};

class C : public A
{
      public:
             C() {}
             ~C() {}
             void wypisz()
             {
                  cout << "C" << endl;
             }
};

class D : public A
{
      public:
             D() {}
             ~D() {}
             void wypisz()
             {
                  cout << "D" << endl;
             }
};

class Alist : public list<A*>
{
      Alist() {}
      ~Alist()
      {
              this->clear();
      }
      Alist& operator=(const Alist& a)
      {
             if (*this != a)
             {
                       this->~Alist();
                       *this = a;
             }
             return *this;
      }
      Alist(const Alist& a)
      {
                  *this = a;
      }
      Alist& operator+(const Alist& a)
      {
             Alist::iterator i;
             for (i = a.begin(); i < a.end(); ++i)
                 this->push_back(*i);
             return *this;
      }
};   

int main()
{
    Alist a;
    
    a.push_back(new B);
    a.push_back(new C);
    
    list<A*>::iterator i;
    for (i = a.begin(); i < a.end(); ++i)
        (*i).wypisz();
    
    system("PAUSE");
    return 0;
}

Z góry dziękuję za wszelkie sugestie :)

A i jeszcze mam taki problemik. W zadaniach zazwyczaj podaja mi, że w funkcji przekazuję const char* podczas gdy w moim typie jest oczywiście char*. Mój kompilator wali błędami jeśli chcę do chara przypisać const char* (invalid conversion from const char to char). Jest na to jakieś proste wyjaśnienie?

Co do pierwszego to nie napisałeś z czym masz konkretny problem.
Co do drugiego to powód jest prosty. Skoro funkcja przyjmuje char* to nie deklaruje, że go nie będzie zmieniać, a const char* nie można zmieniać.

No to zaczynamy:

class Alist : public list<A*>

Nie powinieneś dziedziczyć po STLowych kontenerach, one nie są przystosowane do dziedziczenia (np. nie zawierają wirtualnych destruktorów). Zamiast tego wykorzystuj kompozycję i szablony.

      ~Alist()
      {
              this->clear();
      }

Niepotrzebne, destruktor listy i tak usuwa wszystkie elementy.

    list<A*>::iterator i;

a jest const więc musisz użyć const_iterator zamiast iterator.

    for (i = a.begin(); i < a.end(); ++i)

na iteratorach listy nie można robić porównań < i > tylko != i ==. W tym przypadku powinno być !=.

No i najlepsze na sam koniec:

      Alist& operator=(const Alist& a)
      {
             if (*this != a)
             {
                       this->~Alist();
                       *this = a;
             }
             return *this;
      }

Ten kawałek kodu pokazuje jak mocno można łatwo się pociąć w C++.

if (*this != a)

Jeżeli chodziło ci o uniknięcie przypisania obiektu do samego siebie to powinno być this != &a w warunku.

this->~Alist();

Destruktor to nie jest zwykła metoda którą sobie wywołujesz od tak sobie. Jawne wywołanie destruktora właściwie ma sens tylko prze placement new i tylko wtedy kiedy wiesz co robisz. Chcesz wyczyscic liste? - wywołaj bezpośrednio clear.

*this = a;

Tutaj wchodzisz w nieskończoną rekurencje - jestes w funkcji operator= i wywołujesz samą siebie. Jeżeli chodziło Ci o wywołanie operator= dla klasy bazowej to musisz użyć takiej składni:

list<A*>::operator=(a);

Poza tym można by się jeszcze przyczepić o to, że nie zwalniasz pamięci po new i pewnie jeszcze o kilka innych rzeczy :]

Ok, dzięki :) jakoś słabo poszło, w ogóle nie bardzo wiedziałem jak się za to wziąć ale w zadaniu miałem nakaz dziedziczenia z STL list. Mam jeszcze pytanko jedno, co do zwalniania pamięci, mając jakiś kontener STL (list lub vector) używając push_back tworzona jest kopia czy tylko wskaźnik do elementu? bo jeśli mam załóżmy:

 
vector<int*> vec;
int *temp = new int;
*temp = 3;
vec.push_back(temp);
delete temp;

to mi wyskakują niestworzone liczby, a jak delete usunę to jest ok.

ale w zadaniu miałem nakaz dziedziczenia z STL list

Tak, domyślam się. Jeżeli to tylko ćwiczenie to ok, ale jeżeli wykładowca/nauczyciel twierdzi, że dziedziczenie po kontenerach STL jest poprawne to raczej nie jest zbytnio ogarnięty.

Mam jeszcze pytanko jedno, co do zwalniania pamięci, mając jakiś kontener STL (list lub vector) używając push_back tworzona jest kopia czy tylko wskaźnik do elementu

W przypadku kontener<int*> tworzona jest kopia wskaznika na int, w przypadku kontener<int> jest kopia int.
Tak więc kod:

vector<int*> vec;
int *temp = new int;
*temp = 3;
vec.push_back(temp);
delete temp;

nie jest poprawny. Po delete w wektorze masz wskaznik, ktory wskazuje na zwolnioną pamięć. Stąd masz śmieci.

to zależy bo jak masz tak jak w twoim przykładzie vector<int *> to kompilator wygeneruje listę która będzie zawierać w węźle po prostu wskaźnik. Gdybyśmy dali takie coś: vector<int> to gdybyśmy dali push_back to w wektorze była by kopia elementu które przesłaliśmy to chyba logiczne a tak "wskaźnik na inta z listy" pokazuje na wskaźnik "temp" czyli tak defacto pokazują na ten sam obiekt. gdy skasujemy wskaźnik na obiekt na który pokazywał temp to wskaźnik z listy będzie pokazywał na element który nie istnieje bo przecież wskaźnik z listy i temp pokazywały na ten sam obiekt

Dzięki wielkie za odpowiedzi :) jako że ostatnio dość intensywnie piszę znów przychodzą mi problemy. Jakbyście byli tak mili zerknąć. Mam taki kod, możliwe że już w nim namieszałem z chęci poprawienia go ale myślę że w miarę zachował swoją postać.

 
#include <cstdlib>
#include <iostream>

using namespace std;

template <class T>
class List
{
      struct node
      {
             T val;
             node *next;
             node() {next = NULL;}
             node(T v) {val = v; next = NULL;}
      };
      
      node *first;
      
      public:
             List() {first = NULL;}
             ~List()
             {
                    while (first->next != NULL)
                    {
                          node *temp = first;
                          first = first->next;
						  delete temp;
                    }
                    delete first;
             }
             bool pushFront(const T&);
             bool getLast(T&);
             List<T> operator=(List<T>);
             friend ostream& operator<<(ostream& out, List<T> l)
             {
                    node *temp = l.first;
                    if (temp == NULL)
                       return out;
                    while(temp->next != NULL)
                    {
                              out << temp->val << endl;
                              temp = temp->next;
                    }
                    out << temp->val << endl;
                    return out;
             }
};

template <class T>
bool List<T>::pushFront(const T &t)
{
     if (first == NULL)
     {
               first = new node(t);
               return true;
     }
     node *temp = first;
     while (temp->next != NULL)
     {
       if (temp->val == t)
          return false;
       temp = temp->next;
     }
     temp = new node(t);
     temp->next = first;
     first = temp;
     return true;
}

template <class T>
bool List<T>::getLast(T &t)
{
     node *temp = first;
     if (first == NULL)
        return false;
     while(temp->next != NULL)
		temp = temp->next;
     t = temp->val;
	 return true;
}

template <class T>
List<T> List<T>::operator=(List<T> l)
{
        List<T> temp;
        node *n = l.first;
        if (l.first == NULL)
           return temp;
        while(n->next != NULL)
        {
                  temp.pushFront(n->val);
                  n = n->next;
        }
        temp.pushFront(n->val);
        return temp;
}
                  

int main()
{
    List<int> l;
	int n;
    l.pushFront(4);
    l.pushFront(5);
    l.pushFront(6);
	l.getLast(n);
	cout << n << endl;
	List<int> l1;
	l1 = l;
    cout << l1;
    
    system("PAUSE");
    return 0;
}

Wydaje się, że nie działa tylko operator =, nie wiem jednak o co chodzi ponieważ już żmudnym sposobem wstawiałem co linijke wypisywanie temp i okazuje się że do momentu

 return temp; 

wszystko gra i tem jest jak należy. Ponadto zauważyłem ostatnio, nie tylko w tej aplikacji że już po system("PAUSE")

 program się wiesza. Przekopiowałem do visual studio i wyskoczył mi taki błąd : Debug assertion failed: _BLOCK_TYPE_IS_VALID(phead->nBlockUse). Nie wiecie czym to może być spowodowane, szukałem w google i były problemy typu użycia delete [] T podczas gdy tworzyło się new T. Na ten moment nie mogłem się niczego doszukać ale trochę i tak mam mętlik. Dziwne też że ujawnia się dopiero po <code class="cpp">system("PAUSE")

tak jakby z returnem był problem?

Problem nie jest z returnem, tylko debugger pokazuje, że nie może zejść niżej. Ale to nie znaczy, że wykonywanie programu kończy się na returnie, są wywoływane np. destruktory i na 99% błąd występuje w jednym z nich. Na oko widać na przykład, że destruktor wywali się, gdy lista będzie pusta.

Używaj list inicjalizacyjnych zamiast przypisywać wartości w ciele konstruktora.
Akcesor (get), który zwraca bool i przyjmuje jako argument referencję jest dość nieintuicyjny. Dodatkowo wymusza na użytkowniku, żeby stworzył jakiś obiekt, który i tak zostanie później zastąpiony czymś innym. A co, jeżeli nie będzie mieć publicznego konstruktora bezparametrowego? Nie bój się zwracać obiektów. Z return value optimization i r-value references nie powstaną żadne nadmiarowe kopie.
Dlaczego operator przypisania zwraca zupełnie nowy obiekt? Nie ma tutaj żadnej magii, nie zostanie on automatycznie przypisany do obiektu, na którym go wykonujesz.
Dlaczego wymuszasz unikalność elementów w liście? To jakiś odgórny wymóg?
A co do błędu, powtarzam, szukaj go w destruktorach.

byki to ty masz w tym swoim operatorze przypisania.

template <class T>
List<T>& List<T>::operator=(const List<T> &l)
{
    if(this == &l || !l.first)
        return *this;

    node *n = l.first;
    while(n->next != NULL)
    {
        pushFront(n->val);
        n = n->next;
    }
    pushFront(n->val);
    return *this;
}

edit: ;) no i to co @up

Ten get był zadany taki, tak sam z reszta jak to że elementy musza być unikalne. Takie zadania na egzamin :( Często w ogóle treści nie mogę zrozumieć. Ale co do tego destruktora to nie jestem w stanie się doszukać czegoś. Próbowałem dodać konstruktor do node ale też nic.

Edit: A co do tego rozwiązania operator=, czy przypadkiem on nie nadpisuje mojej listy? Bo nie widzę, żebyś usuwał elementy, które już się tam znajdują?

kidziman napisał(a):

A co do tego rozwiązania operator=, czy przypadkiem on nie nadpisuje mojej listy? Bo nie widzę, żebyś usuwał elementy, które już się tam znajdują?

ja tylko przerobiłem twój kod żeby działał, można w sumie wyczyścić tą listę zanim wywołamy na niej pushFront wtedy będziesz miał kopie tego co przypisujesz bo w takiej formie jak jest teraz to dodaje tylko te elementy. Poza tym przydałoby się zdefiniować jeszcze konstruktor kopiujący w tej twojej klasie.

Rozumiem, trochę te zadania są głupio sformułowane. Wczoraj na przykład znalazłem zadania, w którym miałem napisać szablon tablicy dwuwymiarowej koniecznie używając STL. Mam jednak jeszcze taki pytanko. Mam takie zadanka, w których to mam podany kod i muszę napisać co wypisze i dlaczego. Jeden jest taki:

#include <cstdlib>
#include <iostream>

using namespace std;

class A
{
      public:
             virtual void f() {printf("~A.f() ");}
             virtual ~A() {f();}
};

class B : public A
{
      public:
             void f() {printf("~B.f() ");}
             ~B() {f();}
};

int main()
{
    A *a = new B();
    printf("M ");
    delete a;
    
    system("PAUSE");
    return 0;
}

A drugi taki:

 #include <cstdlib>
#include <iostream>

using namespace std;

class A
{
      public:
             virtual void f() {printf("A.f ");}
             A() {f();}
             ~A() {printf("~A.f() ");}
};

class B : public A
{
      public:
             A *a;
             void f() {printf("B.f ");}
             B() {f(); a = new A(); throw 0;}
};

int main()
{
    try{
        A *a = new B();
        delete a;
    }catch(...) {printf("Exc ");}
    
    system("PAUSE");
    return 0;
}

Napisałem sobie je no i w pierwszym wyjdzie "M ~B.f() ~A.f()" a w drugim "A.f B.f A.f ~A.f() Exc". Może ktoś byłby wytłumaczyć tak dokładnie dlaczego tak bo póki sam próbowałem zrobić to dochodziłem do trochę innych konkluzji. Dzięki :)

W pierwszym, skoro i f() i destruktor są wirtualny to powinien się wyświetlić destruktor dla B (~B.f()), nie wiem tylko skąd potem się wyświetla ~A.f(). W drugim trochę nie wiem dlaczego tak. Trochę jestem skołowany :P

W pierwszym przypadku uruchamia się też destruktor klasy bazowej.

W drugim jest tak: najpierw konstruktor bazowej i pochodnej, potem konstruktor bazowej, bo tworzony jest obiekt a (składnik klasy pochodnej). Następnie w konstruktorze B wywoływany jest wyjątek co powoduje uruchomienie destruktora klasy bazowej. Zauważ, że składnik klasy pochodnej a utworzony w konstruktorze nie jest zwalniany (nie jest wywoływany destruktor B) - podobnie jak zmienna a z main. Dostęp do tych wskaźników został utracony i dojdzie do wycieku pamięci.

Żeby tak się nie stało stosuję się np. technikę RAII.