Problem z wyrzucaniem elementu z listy (biblioteka <list>)

0

Konstruuje kolejny program na uczelnię i mam kilka problemów:

Funkcja WYL_D. Ma ona za zadanie wyładować wszystkie przesyłki z danej dzielnicy tzn wyrzucić z listy wszystkie elementy w których przesylka.dzielnica jest taka sama jak argument funkcji.
Program się kompiluje i generalnie nawet działa, ale z tą funkcją mam problem, nie wyrzuca wszystkich elementów mimo że powinna (tak mi się przynajmniej wydaje). Głowie się i głowie i nie wiem.

Problem mam też z funkcją WYL_P. Jeżeli każę jej wypakować inną niż pierwszą paczkę to wszystko jest ok, jednak przy wypakowywaniu pierwszej nie działa.

Bardzo proszę o pomoc

#include <iostream>    
#include <cstring>
#include <list>
 
using namespace std;
 
struct przesylka
{
    int numer_przesylki;
    char adresat[70];
    char dzielnica[30];
    char towar[30];
    int ciezar;
    int wartosc;
};
 
class KURIER
{
public:
    char NR[30];
    int LAD;
    list <przesylka> PACZKI;
    int IleElementow;
    KURIER();
    KURIER(char *numer, int lad)
    {
        strcpy(NR,numer);
        LAD=lad;
        IleElementow=0;
    }
 
    przesylka TworzPrzesylke(int NUMER, char *ADRESAT, char *DZIELNICA, char *TOWAR, int CIEZAR, int WARTOSC)
    {
        przesylka nowa;
        nowa.numer_przesylki=NUMER;
        strcpy(nowa.adresat,ADRESAT);
        strcpy(nowa.dzielnica,DZIELNICA);
        strcpy(nowa.towar,TOWAR);
        nowa.ciezar=CIEZAR;
        nowa.wartosc=WARTOSC;
        return nowa;
    }
 
    virtual void WLO_Z(przesylka paczka)//wkladanie przesylki
    {
        try
        {
            if(IleElementow>=LAD) throw 0;
 
 
            PACZKI.push_back(paczka);
            IleElementow++;
        }
        catch (int nr)
        {
            if(nr==0)
                cout<<"Nie moge zaladowac, samochod jest pelen"<<endl;
        }
    }
 
    void WYL_P(int ident)//wyladowanie wskazanej identyfikatorem paczki
    {
        bool CzyZrobione=0;
        try
        {
            for(list<przesylka>::iterator it=PACZKI.begin(); it!=PACZKI.end(); it++)
            {
                if((*it).numer_przesylki==ident)
                {
                    PACZKI.pop_back();
                    CzyZrobione=1;
                    break;
                }
            }
            if(CzyZrobione==0)throw 0;
            IleElementow-=CzyZrobione;
        }
        catch (int nr)
        {
            if(nr==0)cout<<"Nie ma takiej paczki"<<endl;
        }
 
    }
 
    bool CzyToSamo(char *a,char *b)
    {
        for(int i=0;i<strlen(a);i++)
        {
            if(a[i]!=b[i])return 0;
        }
        return 1;
    }
 
 
//O TA FUNKCJE MI CHODZI!!!
    void WYL_D(char *dzielnica)//wyladowanie wszystkich z tej samej dzielnicy
    {
        try
        {
            int IleWypakowalo=0;
            for(list<przesylka>::iterator it=PACZKI.begin(); it!=PACZKI.end(); it++)
            {
                if(CzyToSamo(dzielnica,(*it).dzielnica)==1)/
                {
                    PACZKI.pop_back();
                    IleWypakowalo++;
                }
            }
            if(IleWypakowalo==0) throw 0;
            cout<<"Wypakowalem "<<IleWypakowalo<<" paczek"<<endl;
            IleElementow-=IleWypakowalo;
        }
        catch(int nr)
        {
            if(nr==0)cout<<"Nie ma takiej dzielnicy, nic nie wypakowalem"<<endl;
        }
    }
 
 
 
    void Wyswietl()
    {
        int numer=1;
        try
        {
            if(IleElementow==0)throw 0;
            cout<<"Wyswietlam przesylki: "<<endl;
                for(list<przesylka>::iterator it=PACZKI.begin(); it!=PACZKI.end(); it++)
                {
                    cout<<numer<<": "<<(*it).numer_przesylki<<" "<<(*it).adresat<<" "<<(*it).dzielnica<<" "<<(*it).towar<<" masa: "<<(*it).ciezar<<" wart: "<<(*it).wartosc<<endl;
                    numer++;
                }
        }
        catch (int nr)
        {
            if(nr==0)cout<<"Samochod jest pusty, nie mam co wyswietlic"<<endl;
        }
 
    }
 
};
 
int main()
{
    //TworzPrzesylke(int NUMER, char *ADRESAT, char *DZIELNICA, char *TOWAR, int CIEZAR, int WARTOSC)
    KURIER a("PO654SU",3);
    przesylka p1,p2,p3,p4,p5,p6,p7,p8,p9;
    p1=a.TworzPrzesylke(1,"Nowak","Wilda","stol",10,500);
    p2=a.TworzPrzesylke(2,"Kowalski","Wilda","krzeslo",5,100);
    p3=a.TworzPrzesylke(3,"Inne","Wilda","komputer",20,5000);
    p4=a.TworzPrzesylke(4,"Zenon","Grunwald","monitor",3,1000);
    p5=a.TworzPrzesylke(5,"Jan","Grunwald","monitor",3,66);
    p6=a.TworzPrzesylke(6,"Krzysiek","Wilda","monitor",3,3000);
    p7=a.TworzPrzesylke(7,"Adam","Grunwald","monitor",3,1010);
    p8=a.TworzPrzesylke(7,"Adam","Wilda","monitor",3,1010);
 
    cout<<"Wyswietlam na poczatek"<<endl;
    a.Wyswietl();
    cout<<"pakuje 1"<<endl;
    a.WLO_Z(p1);
    cout<<"pakuje 2"<<endl;
    a.WLO_Z(p2);
    cout<<"pakuje 3"<<endl;
    a.WLO_Z(p3);
    cout<<"pakuje 4"<<endl;
    a.WLO_Z(p4);
    cout<<"wyswietlam"<<endl;
    a.Wyswietl();
    cout<<"wypakowuje 2"<<endl;
    a.WYL_P(2);
    a.Wyswietl();
    cout<<"Wypakowuje wilde"<<endl;
    a.WYL_D("Wilda");
    a.Wyswietl();
 
    cout<<endl<<endl;
    system("pause");
    return 0;
}
0

Radzę Ci skasować ten kod i napisać od nowa, zanim ktoś go zobaczy. ;-)

Po co Ci te wyjątki? Wszystko można zastąpić zwykłymi instrukcjami warunkowymi! Funkcja CzyToSamo jest do niczego - od tego jest strcmp (albo zacznij używać std::string). Wartości logiczne w C++ reprezentuje true oraz false. Jeżeli nie modyfikujesz napisu wewnątrz funkcji nie używaj char * tylko const char *. Nie musisz wszędzie pisać (*it). - iterator jest jak wskaźnik, używaj it->.

W tej funkcji, którą Ci chodzi, przechodzisz listę od przodu a usuwasz element z tyłu. To troszkę bez sensu. Powinieneś użyć np. erase. Nie możesz jednak tak po prostu wpisać sobie tego wewnątrz Twojego for, bo będziesz przeskakiwał jeden element przy każdym usuwaniu. Można to zrobić np. tak:

list<przesylka>::iterator it = PACZKI.begin();
while(it != PACZKI.end()) {
  if (CzyToSamo(dzielnica, it->dzielnica)) {
    // `erase` usuwa element `it` i zwraca iterator do kolejnego elementu
    // mozna tez napisac `PACZKI.erase(it++)`;
    it = PACZKI.erase(it);
    ++IleWypakowalo;
  } else {
    ++it;
  }    
}

(http://ideone.com/N8PwG1)

Zapewne reszta kodu, który robi podobne rzeczy jest w ten sam sposób niepoprawna. Nie zapominaj też, że kiedy robisz coś z elementami listy iteratory mogą stracić ważność.

0

// mozna tez napisac PACZKI.erase(it++);
Nie można, bo

Iterators, pointers and references referring to elements removed by the function are invalidated.
All other iterators, pointers and references keep their validity.

Co można rozumieć, że nie wolno użyć wartości it, po to by z niej uzyskać it+1.
W praktyce to zapewne będzie działać, ale lepiej jednak użyć wartości zwracanej przez erase(), przynajmniej masz gwarancję że jest prawidłowa.

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