Suchfunktion in einer Klasse mit mehreren Instanzen

cyborg_beta

Hast du das gemacht, was ich gesagt habe? Sonst brauchen wir hier gar nicht weiterdiskutieren.

@john-0 sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Das Thema war Rule-of-Five.

Nein, war es nicht. Lies noch einmal die Anforderungen.

wob

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Nein, war es nicht. Lies noch einmal die Anforderungen.

Doch, das ist natürlich das Thema. Ein sehr wichtiges sogar, wenn man eigene Listen implementieren will. Es ist Basis dafür, dass der Code korrekt wird. Wenn du Basiswissen ignorierst, wirst du nicht weit kommen. Dass Code korrekt sein soll und keine Speicherlücken haben soll, ist IMMER Teil der Anforderung, auch wenn nicht explizit aufgeschrieben.

*john 0

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Hast du das gemacht, was ich gesagt habe? Sonst brauchen wir hier gar nicht weiterdiskutieren.

Dreimal darfst Du raten, weshalb ich auf das Thema Rule-of-Five verwiesen habe. Die Klasse Node hat ein Speicherloch.

Nachtrag:

g++ -std=c++20 -Wpedantic -Wall -Wextra -g fragender_singly_linked_list_orig.cc -o fragender_singly_linked_list_orig

ergibt

valgrind --leak-check=full ./fragender_singly_linked_list_orig
==3947279== Memcheck, a memory error detector
==3947279== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3947279== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==3947279== Command: ./fragender_singly_linked_list_orig
==3947279== 
1 , 2 , 3.4
4 , 5 , 6.7
2
0

1 , 2 , 3.4
4 , 5 , 6.7
2
1 , 2 , 3.4
1
==3947279== 
==3947279== HEAP SUMMARY:
==3947279==     in use at exit: 32 bytes in 2 blocks
==3947279==   total heap usage: 7 allocs, 5 frees, 74,832 bytes allocated
==3947279== 
==3947279== 16 bytes in 1 blocks are definitely lost in loss record 1 of 2
==3947279==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3947279==    by 0x1092A6: main (fragender_singly_linked_list_orig.cc:82)
==3947279== 
==3947279== 16 bytes in 1 blocks are definitely lost in loss record 2 of 2
==3947279==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3947279==    by 0x1092E0: main (fragender_singly_linked_list_orig.cc:83)
==3947279== 
==3947279== LEAK SUMMARY:
==3947279==    definitely lost: 32 bytes in 2 blocks
==3947279==    indirectly lost: 0 bytes in 0 blocks
==3947279==      possibly lost: 0 bytes in 0 blocks
==3947279==    still reachable: 0 bytes in 0 blocks
==3947279==         suppressed: 0 bytes in 0 blocks
==3947279== 
==3947279== For lists of detected and suppressed errors, rerun with: -s
==3947279== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

DocShoe

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Hast du das gemacht, was ich gesagt habe? Sonst brauchen wir hier gar nicht weiterdiskutieren.

@john-0 sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Das Thema war Rule-of-Five.

Nein, war es nicht. Lies noch einmal die Anforderungen.

Und ich hatte tatsächlich die Hoffnung, dass du Kritik mal ernst nimmst und nicht wieder austeilst, weil du/deine Programmierversuche wieder kritisiert worden sind.

cyborg_beta

Ich glaube, dann fehlt in Node und Data jeweils ein Destruktor. (Mehr nich)

... zumindest, um Valgrind zufriedenzustellen.

DocShoe

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Ich glaube, dann fehlt in Node und Data jeweils ein Destruktor. (Mehr nich)

... zumindest, um Valgrind zufriedenzustellen.

Und die Kopierkonstruktoren/Zuweisungsoperatoren. Du darfst Speicher auf dem Heap nur ein Mal freigeben, das musst du iwie sicherstellen. Und dafür sind Kopierkonstruktoren/Zuweisungsoperatoren da.

Quiche Lorraine

@cyborg_beta
Warum stürzt wohl der folgende Code ab?

MyList GetList()
{
    MyList ml1;

    ml1.insert(new Data(1, 2, 3.4));
    ml1.insert(new Data(4, 5, 6.7));
    return ml1;
}

int main() {
    MyList ml1 = GetList();
    MyList ml2;

    cout << ml1 << ml2 << "\n";
    ml1.filterByWeight(ml2, 3.3, 3.5);
    cout << ml1 << ml2;
    return 0;
}

*john 0

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

... zumindest, um Valgrind zufriedenzustellen.

Vollkommen falscher Ansatz. Es geht darum, dass das Programm nicht leakt. valgrind ist nur ein Mittel zum Zweck, dies festzustellen. Wenn Du Windows nutzt, kannst Du Dir das Intel oneAPI HPC Toolkit kostenlos herunterladen und den Inspector nutzen, um die Speicherlöcher zu finden. Alternativ kannst Du Dir mit Hilfe von VirtualBox eine Linux VM installieren und darin valgrind nutzen. Man sollte bevor man Code publiziert selbst nachschauen, dass der Code auch funktioniert, d.h. unter anderem nicht leakt. Besonders schlecht ist es, wenn man Anfänger mit schlechtem Code „versorgt“.

cyborg_beta

@john-0 Das beantwortet meine Frage aber nicht, ob Valgrind jetzt ruhig ist oder nicht.

Ansonsten wurde doch gesagt, dass die Rule-of-Five für ihn vermutlich zu kompliziert ist, und nicht der Fragestellung angemessen.

Also, es wäre nett, auf meine Frage einfach mit ja/nein zu antworten.

DocShoe

@cyborg_beta

Das trägt doch iwie alles nicht zur Problemlösung bei. In der Aufgabenstellung des TE war doch nie gefordert, eine eigene Liste zu implentieren, das ist doch auf deinem Mist gewachsen.
Ich sehe das so:

Dem TE wurde std::vector vorgeschlagen, das ist der rundrum-sorglos Container, bei dem man sich um nichts weiter mehr kümmern muss. TE kann sich sofort um die eigentliche Aufgabe kümmern, nämlich den Container nach Elementen mit bestimmten Kriterien zu durchsuchen.
Du hast dem TE die Implementierung einer eigenen Liste empfohlen, bei der nach knapp 3 Wochen noch keine fehlerfreie Variante existiert. Wenn TE diesen Weg gehen möchte, muss man ihm alle Konsequenzen dieser Entscheidung zeigen, und auch wie man die dabei entstehenden Probleme löst. Dazu gehört, dass die Liste alle sinnvollen Operationen korrekt implementiert oder sie verbietet, um Fehlbenutzung zu vermeiden. Eine spezielle Liste, die vor Sonderfällen strotzt und zur Fehlbenutzung einlädt, ist für mich keine Lösung. Wenn TE nicht gezeigt wird, in welchen Situationen die Liste Fehler verursacht, nimmt er sie vllt. in sein Repertoire auf und postet in 2 Monaten einen Thread, in dem er sich über seltsames Programmverhalten und Abstürze wundert, weil er diese Liste benutzt.

Auf der einen Seite schlägst du TE vor, eine eigene Liste zu implementieren und im nächsten Satz sagst du, dass du ihn dafür zu unerfahren hältst und die Rule of 3/5 für ihn zu fortgeschritten ist. Also was denn jetzt?

TL;DR: Entweder einen Container der STL, oder eine vollständige, funktionierende Liste, die sich immer korrekt verhält und keine Memoryleaks hinterlässt.

*john 0

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

@john-0 Das beantwortet meine Frage aber nicht, ob Valgrind jetzt ruhig ist oder nicht.

Man sieht es schon am Code der Node-Klasse, dass es ein Problem gibt – zu jedem new gehört auch ein delete. Du delegierst die Verantwortung fürs Löschen der Exemplare der Data-Klasse an die Node-Klasse. Daher muss die Node-Klasse die Rule-of-Five einhalten – tut sie aber nicht, und natürlich gibt es keinerlei Destruktor. Wie soll dann der Code korrekt funktionieren? Die Ausgabe von valgrind habe ich auch nachgetragen, so dass hier niemand behaupten kann valgrind würde kein Problem melden. Natürlich tut es das. Intels Inspector liefert auch das gleiche Ergebnis – Dein Code hat ein Speicherloch.

Ansonsten wurde doch gesagt, dass die Rule-of-Five für ihn vermutlich zu kompliziert ist, und nicht der Fragestellung angemessen.

Es geht nicht mehr um die Fragestellung des OPs alleine sondern um den fehlerhaften Code, den Du hier eingestellt hast. Entweder nutzt Du SmartPointer oder Du behebst die Probleme in Deinem Code. So ist das insbesondere für Anfänger ein Problem, wenn sie so kaputten Code vorgesetzt bekommen.

Du hast aus meinem Code ohne Not den std::unique_ptr<Data> aus der Node-Klasse entfernt, und durch einen normalen Zeiger ersetzt. Obwohl man Dir gerade gezeigt hat, dass man nicht einfach nur einen rohen Zeiger nutzen kann ohne Leaks zu erzeugen, machst Du's trotzdem. C++ verwendet keinen Garbage Collector, so dass das nicht funktionieren kann.

Quiche Lorraine

@cyborg_beta

Warum zeigt mir der folgende Code Datenmüll an?

void FillList(MyList& ml1)
{
    Data d1(1, 2, 3.4);
    Data d2(4, 5, 6.7);

    ml1.insert(&d1);
    ml1.insert(&d2);

}

int main() {
    MyList ml1, ml2;
    
    FillList(ml1);
    cout << ml1 << "\n";
    return 0;
}

cyborg_beta

@Quiche-Lorraine Deine Gegenfragen haben doch nix mit dem Thema zu tun ... aber gut ...

In der Funktion GetList wird eine Liste erzeugt, mit Elementen gefüllt und zurückgegeben. Der Pointer darauf ist anschließend stale.

Hier steht mehr darüber: https://www.tutorialspoint.com/cplusplus/cpp_return_pointer_from_functions.htm

Quiche Lorraine

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

In der Funktion GetList wird eine Liste erzeugt, mit Elementen gefüllt und zurückgegeben. Der Pointer darauf ist anschließend stale.

Da ist aber kein Pointer, sondern ich möchte hier eine Kopie zurückgeben.

Deine Gegenfragen haben doch nix mit dem Thema zu tun ... aber gut ...

Oh doch, mit Beachtung der Rule of Five wäre das nicht passiert.

#include <list>
#include <algorithm>
#include <iostream>
#include <ostream>

class Car
{
public:
    int topspeed;
    int horsepower;
    double weight;

    Car() : topspeed(-1), horsepower(-1), weight(-1) {}
    Car(int topspeed, int horsepower, double weight) : topspeed(topspeed), horsepower(horsepower), weight(weight) {};

    friend std::ostream& operator<< (std::ostream& o, const Car& d) {
        o << d.topspeed << " , " << d.horsepower << " , " << d.weight << "\n";
        return o;
    }
};

std::list<Car> GetList()    // nur zur Demo
{
    std::list<Car> ml1;

    ml1.push_back(Car(1, 2, 3.4));
    ml1.push_back(Car(4, 5, 6.7));
    return ml1;
}


int main() 
{
    std::list<Car> ml1 = GetList();
    
    std::for_each(std::begin(ml1), std::end(ml1), [](const Car& c) {
        std::cout << c;
        });
    return 0;
}

Aber aus irgenteinem Grund scheint dich dies nicht zu interresieren.

SeppJ

@Quiche-Lorraine sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Aber aus irgenteinem Grund scheint dich dies nicht zu interresieren.

Klar, er wird doch zur Belohnung gut gefüttert.

cyborg_beta

@SeppJ sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Klar, er wird doch zur Belohnung gut gefüttert

Wenn ihr weiter so gehässig sein wollt, macht eben ohne mich weiter

*john 0

@cyborg_beta sagte in Suchfunktion in einer Klasse mit mehreren Instanzen:

Wenn ihr weiter so gehässig sein wollt, macht eben ohne mich weiter

Von Dir sind Null Bemühungen zu erkennen Deinen kaputten Code zu korrigieren. Wenn Du ernstgenommen werden willst, solltest Du das ändern.

cyborg_beta

Ich setzte mich später noch mal dran. Ich will ja meinen guten Willen zeigen ... auch, wenn ich das Gefühl habe, dass ich damit (alleine) nicht weiterkomme.