przeciążanie operatorów- lista

0

Witam, mam problem z przeciążaniem operatorów. Popatrzmy na main - > jest tam kilka rzeczy prezentujących to co się dzieje w klasie Lista. problem mam z operatorem "=". jak widać, w mainie są 2 instrukcje: cout<<"lista 3=1+2: "<<a+b<<endl; oraz c=a+b; pierwsza działa bezproblemowo, jednak na drugiej program się zawiesza (SIGSEGV). po puszczeniu debuggera problem wykrywa w 141 linijce (while (pom->next)). dodatkowa rzecz: mam znaczniki cout<<"A" i cout<<"B" w operatorze + zeby widziec co się dzieje, i jesli uruchomimy program z w.w. instrukcjami, i powiedzmy wpiszemy do 1 listy 2 elementy a do drugiej 3 to pokazuje "AABBB" i "BBB", a powinno "AABBB" "AABBB"; nie mam pojęcia gdzie może być błąd. jeśli ktoś mógłby pomóc to byłbym bardzo wdzięczny :)

/*
1) klasa lista
2) operatory >>(dopisuje 1 element),<<,+(dodaje listy),=(przepisuje wartosci)
3) konstruktory, destruktor (nie stosowac rekurencji)
*/

#include <iostream>
#include "operatory.h"
using namespace std;
template <class T>
class Lista;

template <class T>
class element{
public:
    //friend class Lista; //dzieki temu mamy dostep do prywatnych el. class lista
    T wart;
    element<T>* next;
};

template <class T>
class Lista {
public:
    //friend class element;
    element<T>* head;
    Lista(){
        head=NULL;
    }
    Lista<T> & operator = (Lista <T> o1);

    Lista<T> & operator + (Lista<T> o1);
    //friend istream & operator >> (istream & s, Lista <T> & l);
    //friend ostream & operator << (ostream & s, Lista <T> & l);


    ~Lista(){

        element<T> *pom=head;
        element<T> *pom2;

        while (pom)
        {
            cout<<endl<<"zwolniony adres: "<<pom;
            pom2=pom->next;
            delete pom;
            pom=pom2;
        }
        cout<<endl<<endl;

    }

};


template <class T>
istream & operator >> (istream & s, Lista <T> & l){
    element<T> *nowy,*o2=l.head;
    nowy=new element<T>;
    nowy->next=NULL;
    s>>nowy->wart;
    cout<<endl<<"zapisano element pod adresem "<<nowy<<endl<<endl;
    if (l.head==NULL) {
        l.head=nowy;
    }
    else {
        while (o2->next){
            o2=o2->next;
        }
        o2->next=nowy;
    }
    return s;
}

template <class T>
ostream & operator << (ostream & s, Lista<T> & l){
    element<T> *o1=l.head;
    if (l.head==NULL) s<<"brak elementow";
    while (o1){
        s<<o1->wart;
        o1=o1->next;
    }
    return s;
}


template <class T>
Lista<T> & Lista<T>::operator = (Lista<T> o1){
    static Lista<T> nowa;
    element<T> *pom=nowa.head;
    while (o1.head){
        element<T>* nowy=new element<T>;
        nowy->wart=o1.head->wart;
        nowy->next=NULL;
        if (o1.head->next)

            o1.head=o1.head->next;
        if (!nowa.head)
            continue;
        pom->next=nowy;
        cout<<"C";
    }


    return nowa;
}

template <class T>
Lista<T> & Lista<T>:: operator + (Lista<T> o1){
    static Lista<T> nowa;
    nowa.head=NULL;

    while (head){

        element<T>* nowy=new element<T>;
        element<T>* pom=nowa.head;
        nowy->wart=head->wart;
        nowy->next=NULL;
        if (nowa.head==NULL){
            nowa.head=nowy;
        }

        else{
            while (pom->next){
                pom=pom->next;
            }
            pom->next=nowy;
        }

        head=head->next;
        cout<<"A";
    }
    while (o1.head){

        element<T>* nowy=new element<T>;
        element<T>* pom=nowa.head;
        nowy->wart=o1.head->wart;
        nowy->next=NULL;
        if (nowa.head==NULL)
            nowa.head=nowy;
        else {
            while (pom->next)
                pom=pom->next;

            pom->next=nowy;
        }

        o1.head=o1.head->next;
        cout<<"B";
    }


    return nowa;
}



int main (){

    {

    Lista<int> a,b,c;
    cout<<"ile elementow listy chcesz wpisac?: ";
    int ile_elementow;
    cin>>ile_elementow;
    cout<<"wpisz elementy: ";
    for (int i=0;i<ile_elementow;i++){
        cin>>a;
    }

    cout<<"ile elementow chcesz wpisac?: ";
    cin>>ile_elementow;
    cout<<"wpisz elementy: ";
    for (int i=0;i<ile_elementow;i++){
        cin>>b;
    }
    cout<<"lista 1: "<<a;
    cout<<endl;
    cout<<"lista 2: "<<b;
    cout<<endl;
    cout<<"lista 3=1+2: "<<a+b<<endl;
    c=(a+b);
    cout<<"lista 3=1+2: "<<c<<endl;
    c=a+b;
    cout<<"po ponownym dodaniu: "<<c<<endl;


    /*Lista <macierze> a,b;
    cout<<"wpisz 1 macierz (pierwszy wymiar): ";
    cin>>a;
    cout<<"wpisz 2 macierz (pierwszy wymiar): ";
    cin>>a;
    cout<<a;*/






    //c=(a+b);
    //cout<<endl<<"lista 3=1+2: ";



    //c=(pt[0]+pt[1]);
    //cout<<"po ponownym dodaniu: ";
    //cout<<c;

  }

    return 0;
}
5

Poczytaj o przeładowywaniu operatorów, bo średnio to rozumiesz: http://en.cppreference.com/w/cpp/language/operators

  1. Zwracasz referencje do zmiennych lokalnych. To UB (to jest UB, ale do zmiennych statycznych już nie - mimo że wciąż niezbyt sensowne).
  2. operator+ powinien być realizowany w oparciu o operator+= i powinien zwracać nowy obiekt, a nie referencję.
  3. operator= powinien - odwrotnie - zwracać referencję do danej instancji klasy, a nie pracować na nowej.
  4. używasz nagiego new i delete. To anty-idiom w C++, masz kontenery, masz mądre wskaźniki. Nawet jeśli piszesz własny kontener, nic nie stoi na przeszkodzie aby użyć unique_ptr.
  5. funkcje odpowiedzialne za coś innego - np. operator+ - nie powinny nic wypisywać na ekran.
  6. używaj nullptr zamiast NULL.

tyle na szybko

0
kq napisał(a):

Poczytaj o przeładowywaniu operatorów, bo średnio to rozumiesz: http://en.cppreference.com/w/cpp/language/operators

  1. Zwracasz referencje do zmiennych lokalnych. To UB.
  2. operator+ powinien być realizowany w oparciu o operator+= i powinien zwracać nowy obiekt, a nie referencję.
  3. operator= powinien - odwrotnie - zwracać referencję do danej instancji klasy, a nie pracować na nowej.
  4. używasz nagiego new i delete. To anty-idiom w C++, masz kontenery, masz mądre wskaźniki. Nawet jeśli piszesz własny kontener, nic nie stoi na przeszkodzie aby użyć unique_ptr.
  5. funkcje odpowiedzialne za coś innego - np. operator+ - nie powinny nic wypisywać na ekran.
  6. używaj nullptr zamiast NULL.

tyle na szybko

dzięki za te wszystkie wskazówki, jestem pewien że się przydadzą jednak mam ten jeden konkretny problem i to co napisałeś w żaden sposób mi w tym problemie nie pomaga. poza tym: 1) zwracam referencję do obiektu lokalnego statycznego, a nie lokalnego. to duża różnica, 2) zgadza się, nie jestem zaawansowany ale myślę że w tego typu programach to czy używam NULL czy nullptr nie robi większej różnicy, podobnie jak wypisywanie na czegoś na ekran przez operator jak i na tym poziomie nie robi dużej różnicy czy używam "gołego" new i delete czy czegoś innego. ale jeszcze raz, dzieki za wskazówki, uczę się:)

2
  1. Racja, przeoczyłem to. Ale teraz to ma jeszcze mniej sensu.
  2. Ale po co uczyć się złych nawyków?
  3. Przejrzałem operator= - gdzie przypisujesz elementy do nowa?
  4. operator+ przejeżdża wskaźnikiem na koniec listy, wobec czego po jego wykonaniu jest ona efektywnie pusta.
cout << a+b << endl;
cout << a+b << endl;

wyświetli różne wyniki.

3
mpiwosal napisał(a):
  1. zwracam referencję do obiektu lokalnego statycznego, a nie lokalnego. to duża różnica
    A po co zwracasz referencję? Czemu to ma służyć? I tak potem przypisując do c robisz kopię. Co gorsza nie pozwalasz kompilatorowi na zastosowanie optymalizacji typu NRVO.
  1. zgadza się, nie jestem zaawansowany ale myślę że w tego typu programach to czy używam NULL czy nullptr nie robi większej różnicy, podobnie jak wypisywanie na czegoś na ekran przez operator jak i na tym poziomie nie robi dużej różnicy czy używam "gołego" new i delete czy czegoś innego. ale jeszcze raz, dzieki za wskazówki, uczę się:)
    Dużo łatwiej jest wyrobić dobre nawyki gdy się pisze małe programy, niż gdy już masz złe nawyki utrwalone i idziesz do pracy.
0
kq napisał(a):
  1. Racja, przeoczyłem to. Ale teraz to ma jeszcze mniej sensu.
  2. Ale po co uczyć się złych nawyków?
  3. Przejrzałem operator= - gdzie przypisujesz elementy do nowa?
  4. operator+ przejeżdża wskaźnikiem na koniec listy, wobec czego po jego wykonaniu jest ona efektywnie pusta.
cout << a+b << endl;
cout << a+b << endl;

wyświetli różne wyniki.

  1. mniej sensu? czemu?
  2. uczę się podstaw. dosyć ciężko zaczynać od szczegółów
  3. hmm... a muszę? "nowa" to na początku tylko pusty wskaźnik "head" na który ustawiam wskaźnik pomocniczy z którego później korzystam->
element<T> *pom=nowa.head;
  1. racja, to przeoczyłem.
o1.head

moze zostac ale this->head

 : nie.

dzięki, jakbyś miał jeszcze jakieś wskazówki to chętnie przyjmę :)
0
  1. Utrudniasz wielowątkowość, wprowadzasz zbędny stan globalny, a nawet wywołania typu zaczynają dawać głupie rezultaty
void foo(Lista<int> const& l, Lista<int> const& r);

foo(L1+L2, L3+L4);
  1. na geografii nie zaczynasz nauki od tego, że Ziemia jest płaska, żeby potem przejść do "szczegółów". Na matematyce kolejności działań nie wprowadzają w liceum (szczegóły, co?).
int head = 0;
int pom = head;
pom = 42;

czy zmieniłeś head?
4) Innej listy też nie możesz przesuwać.

0
twonek napisał(a):
mpiwosal napisał(a):
  1. zwracam referencję do obiektu lokalnego statycznego, a nie lokalnego. to duża różnica
    A po co zwracasz referencję? Czemu to ma służyć? I tak potem przypisując do c robisz kopię. Co gorsza nie pozwalasz kompilatorowi na zastosowanie optymalizacji typu NRVO.
  1. zgadza się, nie jestem zaawansowany ale myślę że w tego typu programach to czy używam NULL czy nullptr nie robi większej różnicy, podobnie jak wypisywanie na czegoś na ekran przez operator jak i na tym poziomie nie robi dużej różnicy czy używam "gołego" new i delete czy czegoś innego. ale jeszcze raz, dzieki za wskazówki, uczę się:)
    Dużo łatwiej jest wyrobić dobre nawyki gdy się pisze małe programy, niż gdy już masz złe nawyki utrwalone i idziesz do pracy.

zwracam referencję dlatego że operator

istream & operator >> (istream & s, Lista <T> & l);

oczekuje referencji. Mógłbym to też zmodyfikować ale wtedy program za każdym razem musiałby kopiować listę do funkcji, nie?

0

Nic bardziej mylnego. Dowolne lvalue tam pasuje. http://en.cppreference.com/w/cpp/language/value_category

0
kq napisał(a):
  1. Utrudniasz wielowątkowość, wprowadzasz zbędny stan globalny, a nawet wywołania typu zaczynają dawać głupie rezultaty
void foo(Lista<int> const& l, Lista<int> const& r);

foo(L1+L2, L3+L4);
  1. na geografii nie zaczynasz nauki od tego, że Ziemia jest płaska, żeby potem przejść do "szczegółów". Na matematyce kolejności działań nie wprowadzają w liceum (szczegóły, co?).
int head = 0;
int pom = head;
pom = 42;

czy zmieniłeś head?

  1. Innej listy też nie możesz przesuwać.
  1. masz rację.
  2. wiem że nie, ale nie potrafię sobie wyobrazić jak to ma działać w tym przypadku. mógłbyś mi pokazać co mam zrobić w tym operatorze?
  3. w tym przypadku mogę przesuwać head, bo przekazałem kopię listy. chyba :D
0
  1. tak jak w operatorze+ (nie nadpisujesz pom, tylko pom->next, co jest na początku równoznaczne z head->next)
  2. fakt. Ale to kolejny błąd (zobacz pierwszy link o przeładowywaniu operatorów)
0
mpiwosal napisał(a):

zwracam referencję dlatego że operator

istream & operator >> (istream & s, Lista <T> & l);

oczekuje referencji.
Nie nie nie. Ten operator przekazuje parametr przez referencję, co absolutnie nie oznacza tego co piszesz. Polecam lekturę pomocniczą: Przekazywanie parametru przez wartość i referencję
W tym przypadku żeby było w pełni poprawne bo powinien przekazać przez stałą referencję, bo nie powinieneś modyfikować argumentu.

Lista<T> & Lista<T>::operator = (Lista<T> o1)

Tutaj przekazujesz parametr przez wartość, przez co funkcja robi niepotrzebnie kopię (chyba że kompilator był na tyle sprytny, że to zoptymalizował). Powinno być

Lista<T>& Lista<T>::operator=(const Lista<T>& o1)
0

nie rozumiem czemu mi się to wykonuje w nieskonczonosc...

template <class T>
Lista<T>  Lista<T>::operator = (Lista<T> o1){
    Lista<T> nowa;
    element<T> *pom=o1.head;
    element<T> *pom2=nowa.head;
    while (pom){
        element<T>* nowy=new element<T>;
        nowy->wart=pom->wart;
        pom=pom->next;
        if (!nowa.head){
            nowa.head=nowy;
            continue;
        }
        pom->next=nowy;
    }

    return nowa;
}
0

Bo zawsze ustawiasz pom->next na nowy, wobec czego po przejściu do następnego elementu w pom=pom->next jest on różny od nullptr. Poza tym, jeśli tylko sprawdziłeś pom to nie powinieneś operować na pom->next, to jest potencjalnie niebezpieczne.

0
kq napisał(a):

Bo zawsze ustawiasz pom->next na nowy, wobec czego po przejściu do następnego elementu w pom=pom->next jest on różny od nullptr. Poza tym, jeśli tylko sprawdziłeś pom to nie powinieneś operować na pom->next, to jest potencjalnie niebezpieczne.

poprawione, ale dalej źle:

Lista<T> & Lista<T>::operator = (Lista<T> o1){
    static Lista<T> nowa;
    element<T> *pom=o1.head;
    element<T> *pom2=nowa.head;
    while (pom){
        element<T>* nowy=new element<T>;
        nowy->wart=pom->wart;
        pom=pom->next;
        if (!nowa.head){
            nowa.head=nowy;
            continue;
        }
        pom2->next=nowy; //tu wywala
        pom2=pom2->next;
    }

    return nowa;
}

nie powinienem operowac na pom->next?
to jak inaczej "powiązać listę?"

0

Możesz, ale jeśli sprawdzisz, czy pom->next jest różne od nullptr (za wyjątkiem przypisania do next)

0
kq napisał(a):

Możesz, ale jeśli sprawdzisz, czy pom->next jest różne od nullptr (za wyjątkiem przypisania do next)

ok. a co do tego czemu mi wywala? jakiś pomysł?

1 użytkowników online, w tym zalogowanych: 0, gości: 1