Lista jednokierunkowa - nieznany błąd uruchomienia...

Mam zadanie: W istniejącej liście jednokierunkowej: jeśli średnia jest mniejsza od liczby T= 9 (tak dobrać wartość by warunek był prawdziwy ;) usuń z listy drugi i ostatni element. Zwrócić usunięte elementy przez parametry, zwrócić średnią (return), do listy na końcu dodać element będący sumą wcześniej usuniętych dwóch elementów.

Coś jednak nie działa. Wyskakuje w Windowsach komunikat "program przestał działać" (wykorzystuje Code::Blocks). Nie wiem w czym problem...

Z góry dziękuje za wskazanie błędu(ów). Ja nie mogę się doszukać. Wydaje się, że nie działa dodanie elementu na koniec listy (którego wartość równa jest sumie: skas1 + skas2).

Tak jak pisałem program się kompiluje, ale "wykrzacza" się w pewnym momencie :(

DZIEKUJE ZA POMOC!

#include <iostream>

using namespace std;


struct Wezel {
   int dane;
   Wezel * nast;
};

float FunkcjaZ1(Wezel * glowa, int &skas1, int &skas2, int T);
void addFront(Wezel *&poczatek, int dane);


int main()
{

    Wezel * poczatek = NULL;

    addFront(poczatek, 12);
    addFront(poczatek, 10);
    addFront(poczatek, 2);
    addFront(poczatek, 6);
    addFront(poczatek, 4);
    addFront(poczatek, 5);
    addFront(poczatek, 7);
    addFront(poczatek, 3);
    addFront(poczatek, 8);
    addFront(poczatek, 1);
    addFront(poczatek, 9);
    addFront(poczatek, 11);
    addFront(poczatek, 14);

    int wartoscSkasowana1 = 0;
    int wartoscSkasowana2 = 0;
    float srednia;

    srednia = FunkcjaZ1(poczatek, wartoscSkasowana1, wartoscSkasowana2, 8);

    cout << "Wartosci skasowane:" << endl;

    cout << wartoscSkasowana1 << endl;
    cout << wartoscSkasowana2 << endl;
    cout << "Wartosc srednia: " << srednia << endl;
    return 0;
}

float FunkcjaZ1(Wezel * glowa, int &skas1, int &skas2, int T)
{
    Wezel * p = glowa;
    while(p)
    {
        cout << p->dane << endl;
        p = p->nast;
    }

    p = glowa;
    int ileEl = 0;
    int sumaEl = 0;
    float srednia = 0.0;

    while(p)
    {
        ileEl++;
        sumaEl+= p -> dane;
        p = p->nast;
    }

    cout << "Liczba elementow: " << ileEl << endl;
    cout << "Suma elementow: " << sumaEl << endl;


    if (ileEl > 0)
    {
        srednia = (float)sumaEl / ileEl;
    }

    cout << "Srednia z elementow: " << srednia << endl;

    if ((srednia < T) && (ileEl >= 3))
    {
        p = glowa;
        Wezel * tmp;
        tmp = p -> nast;
        p->nast = tmp -> nast;

        skas1=tmp->dane;
        cout << "--kasowany element 2: " << skas1<< endl;
        delete tmp;
        ileEl--;

        p = glowa;
        for (int i=0; i<ileEl-2; i++)
        {
            p = p->nast;
        }

        tmp = p -> nast;
        skas2 = tmp->dane;
        cout << "--kasowany element ostatni: " << skas2<< endl;
        delete tmp;
        p->nast = NULL;

        cout << endl << " PO SKASOWANIU OBU ELEMENTOW: " << endl;

        p = glowa;
        while(p)
        {
            cout << p->dane << endl;
            cout << p->nast << endl;
            p = p->nast;
        }


        Wezel * nowyEl = new Wezel;

        p=glowa;
        while(p)
        {
            p= p->nast;
        }
        p -> nast = nowyEl;

        nowyEl->dane = skas1 + skas2;
        nowyEl->nast = NULL;

    }

    return srednia;
}


void addFront(Wezel *&poczatek, int dane)
{
    Wezel * tmp;
    tmp = new Wezel;
    tmp->dane = dane;
    tmp->nast = poczatek;
    poczatek = tmp;
}

Debugger w rękę i do dzieła. Swoją drogą gdybyś użył std::unique_ptr zamiast nagich wskaźników i new i delete to by było znacznie czytelniej i prościej.

Debugger w Code::Blocks? Przy próbie debugowania pyta mnie o Visual Studio (którego nie mam i korzystam z modemu 3G więc nawet nie będę próbował ściągać ;) ).

Tych konstrukcji, nowszych od new, delete nie znam. Na zajęciach nie wychodzimy poza standard C++ 98 (??), czyli to co opisuje Grębosz u siebie.

Co jest nie tak z new, delete?? Tak jak pisałem obowiązującą u nas "lekturą" jest Grębosz.

• Masz wycieki pamięci, bo nie zwalniasz pamięci po obiektach;
• Twoja funkcja FunkcjaZ1 robi tyle rzeczy, że masakra. Podziel zadania na mniejsze funkcje;
• Robisz z alokacją pamięci straszne cuda. Najlepiej w ogóle to napisać od nowa.

A to o czym pisał @kq pozwoli Ci zapomnieć o problemach zwalniania zasobów ręcznie etc. Grębosz dzisiaj to już strasznie przestarzała lektura.

Jak już ma być po staremu:

#include<iostream>
using namespace std;

struct node
{
    int number;
    node *next;
    node(int number) : number(number), next(nullptr){}
};

void add(node *&head, int number)
{
    node *element = new node(number);
    element->next = head;
    head = element;
}

void remove(node *head)
{
    node *temp = head;
    while(temp)
    {
        node *current = temp;
        temp = temp->next;
        delete current;
    };
}

int main()
{
    node *list = nullptr;
    add(list, 1);
    add(list, 2);
    add(list, 3);
    remove(list);
    return 0;
}

Zainwestuj w jakąś wirtualkę z Linuksem i postaw sobie na niej valgrind'a. Pozwoli Ci to połapać wycieki pamięci czy mazanie po nie swoich zasobach.

Wedle szybkiego googlowania Code::Blocks jak najbardziej wspiera debuggery.

Co do new i delete: ich używanie to antyidiom w ("nowoczesnym"¹) C++.

1. ręczne zarządzanie pamięcią - musisz pamiętać żeby wszystko pozwalniać, również w mniej oczywistych przypadkach wyjścia z zakresu (np. rzucenie wyjątku). C++ ma RAII nie bez powodu.
2. ujemna czytelność kodu - trudne rozróżnienia pointera obserwującego od posiadającego (masz T* t. Kto odpowiada za wykonanie delete na t?)

Co do Symfonii to współczuję, tutaj szerzej opisane: http://www.wykop.pl/wpis/9719000/cpp-naukaprogramowania-programowanie-wstep-czestot/

¹ co najmniej od 2011r. Realistycznie dobre kilka lat wcześniej. Na chwilę pisania tego posta mamy rok 2016.

Masz wycieki pamięci, bo nie zwalniasz pamięci po obiektach;

Tzn. Masz na myśli, to, że na końcu nie usuwam całej listy? Czy jakieś inne niedopatrzenia?

Twoja funkcja FunkcjaZ1 robi tyle rzeczy, że masakra. Podziel zadania na mniejsze funkcje;

W zadaniu sugerują wykonać to w jednej funkcji, ale chyba rzeczywiście to podzielę.

Robisz z alokacją pamięci straszne cuda. Najlepiej w ogóle to napisać od nowa.

Tak też zrobię,

A to o czym pisał @kq pozwoli Ci zapomnieć o problemach zwalniania zasobów ręcznie etc.

To jest jakiś nowy garbage collector w nowym/nowszym C++ ?? Czy jakieś inteligentne pointery?

Funkcja usuwająca całą listę powinna być osobno. Kiedy skończysz działania na liści to usuwasz jej elementy. Patrz mój kod.
Całą resztę natomiast determinuje rozbicie tej God-Function na mniejsze dzieciątka :)

masz T* t. Kto odpowiada za wykonanie delete na t?

Nie nie mam, to jest integer.

node(int number) : number(number), next(nullptr){}

To jeszcze chyba nie jest po staremu :) OK nie będe mieszał.

Rozbije kod na małe, dobrze działające (przetestowane) funkcje...... Ale to już jutro.

DZIĘKI ZA PODPOWIEDZI!

Link na wykopie też jest ciekawy. Cholera może kiedyś się nauczę c++. Z jednej strony bym chciał, a z drugiej trochę mało (i chyba coraz mniej) jest pracy (w porównaniu do mainstreamu) w Cpp.

Cholera może kiedyś się nauczę c++

C++ nie da się nauczyć, uwierz mi. Jest to problem NP-zupełny :)

Wiesz, nie od razu Radom zbudowano!

Zacznę od małych kroków.