Klassenfrage :)

Quiche Lorraine

was ist anderst daran, ausser das ich es nicht kenne?

Naja, das Schlüsselwort new allokierst du eine Ressource. Doch wer gibt diese wieder frei?

Wenn du diese Frage nicht sauber beantworten kannst, bist du auf dem besten Weg ein Speicherloch in dein Programm zu reißen. Und dann crasht dein Programm im besten Fall. Im schlimmsten Fall dein Rechner, weil dein Program auf einmal mehrere Gigabyte RAM frisst.

Deswegen nutzt man die RAII Technik:

https://en.cppreference.com/w/cpp/language/raii

Kein malloc, new,... sondern nur std::unique_ptr, std::vector,...

EL europ

@Quiche-Lorraine
Da triffst du den Nagel zumindest nah am Kopf:
Ich allociere in einer Klasse c-arrays, wenn ich Objekte dieser Klasse in std::vector speichere und diese Arrays im Destruktor mit free() freigebe, funktionieren entsprechende Methoden der Objekte in std::vector nicht mehr. Ich geb zu das die c-arrays und c-threads "quick n dirty" Lösungen sind weil ich meine thread-Lösung nicht einfach in c++ threads umgesetz bekam, wie den Rest des Codes

Quiche Lorraine

@EL-europ sagte in Klassenfrage :

bei Interesse: https://github.com/momefilo/mandelbrodt. Ist aber alles nackter Code ohne Kommentare

Sorry, wenn ich das mal sage. Aber dein Code is grausam:

Du nutzt ungeniert new. Wer gibt myApples.push_back(*new _AppleData); wieder frei`?
Warum allokierst du eigentlich ständig Sachen, welche doch auch auf dem Stack liegen könnten?
Wann darf ein Pointer bei dir NULL sein? Warum nutzt du kein nullptr?
Du missachtest komplett die Rule of Five! (https://en.cppreference.com/w/cpp/language/rule_of_three)
-> Du musst die Move Semantik beachten! (https://en.cppreference.com/w/cpp/language/move_constructor)
Warum nutzt du malloc und nicht std::vector?
Warum hast du einen Quicksort geschrieben und nutzt nicht std::sort?
char iterText[17]; printf(iterText, "I-Werte % 8d", countsOfIter); -> Das ist unter C++ ein völlig veralteter Programmierstil und eine potenzielle Buffer Overflow Sicherheitslücke. Nutze std::format!

Ein paar Cppcheck Sachen:

Apple.cpp Line 127: Common realloc mistake: 'iterMembers' nulled but not freed upon failure
Apple.cpp Line 65: '(void*)tmp_x' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. Arithmetic operations on 'void *' is a GNU C extension, which defines the 'sizeof(void)' to be 1.
Apple.cpp Line 182: Member variable '_Apple::xpos' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior.
_Userinterface.cpp Line 152: Buffer is accessed out of bounds: text
_Userinterface.cpp Line 8: When an object of a class is created, the constructors of all member variables are called consecutively in the order the variables are declared, even if you don't explicitly write them to the initialization list. You could avoid assigning 'callback' a value by passing the value to the constructor in the initialization list.

Das ist jetzt leider für dich ein wenig heftig und das tut mir auch Leid. Aber lerne daraus!

Vergiss das bisher gelernt und lerne aus einem richtig gutem C++ Buch.

EL europ

@Quiche-Lorraine sagte in Klassenfrage :

@EL-europ sagte in Klassenfrage :

bei Interesse: https://github.com/momefilo/mandelbrodt. Ist aber alles nackter Code ohne Kommentare

Sorry, wenn ich das mal sage. Aber dein Code is grausam:

Du nutzt ungeniert new. Wer gibt myApples.push_back(*new _AppleData); wieder frei`?

Warum allokierst du eigentlich ständig Sachen, welche doch auch auf dem Stack liegen könnten?

Wann darf ein Pointer bei dir NULL sein? Warum nutzt du kein nullptr?

Du missachtest komplett die Rule of Five! (https://en.cppreference.com/w/cpp/language/rule_of_three)
-> Du musst die Move Semantik beachten! (https://en.cppreference.com/w/cpp/language/move_constructor)

Warum nutzt du malloc und nicht std::vector?

Warum hast du einen Quicksort geschrieben und nutzt nicht std::sort?

char iterText[17]; printf(iterText, "I-Werte % 8d", countsOfIter); -> Das ist unter C++ ein völlig veralteter Programmierstil und eine potenzielle Buffer Overflow Sicherheitslücke. Nutze std::format!

Ein paar Cppcheck Sachen:

Apple.cpp Line 127: Common realloc mistake: 'iterMembers' nulled but not freed upon failure

Apple.cpp Line 65: '(void*)tmp_x' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined. Arithmetic operations on 'void *' is a GNU C extension, which defines the 'sizeof(void)' to be 1.

Apple.cpp Line 182: Member variable '_Apple::xpos' is not initialized in the constructor. Member variables of native types, pointers, or references are left uninitialized when the class is instantiated. That may cause bugs or undefined behavior.

_Userinterface.cpp Line 152: Buffer is accessed out of bounds: text

_Userinterface.cpp Line 8: When an object of a class is created, the constructors of all member variables are called consecutively in the order the variables are declared, even if you don't explicitly write them to the initialization list. You could avoid assigning 'callback' a value by passing the value to the constructor in the initialization list.

Das ist jetzt leider für dich ein wenig heftig und das tut mir auch Leid. Aber lerne daraus!

Vergiss das bisher gelernt und lerne aus einem richtig gutem C++ Buch.

Ich hab as prog von c in c++ umgeschrieben
zu Apple.line 127: Wenn ich den Arrays zuvor kein NULL zuweise zeigt es zur Laufzeit Speicherfehler.
Apple.line 65: Da meckert der Compieler weil er die Größe des Integerwertes nicht kennt, aber das Programm in der thFunc kennt sie
Apple.line 182: erst nach der Initialisierung durch init() ist die xpos berechnet, und wird zur laufzeit geändert.
userinterface 152: Da schneidet er die restlichen Stellen einfach ab, oder?
userinterface line8: Kann ich nicht nachvollziehen, es geschieht doch die Zuweisung im Konstruktor

Doch danke für deine Hinweise, da hab ich noch ne compilermeldung:
include/_Colorinterface.cpp:245:86: warning: narrowing conversion of ‘(((_Colorinterface*)this)->_Colorinterface::elements.std::vector<_CiElement>::size() - 1)’ from ‘std::vector<_CiElement>::size_type’ {aka ‘long unsigned int’} to ‘int’ [-Wnarrowing]
Wenn ich versuche das letzte Objekt im std::vector mit der array.end() "-Funktion" anzusprechen, compiliert er mir nicht.

Jockelx

@EL-europ sagte in Klassenfrage :

userinterface line8: Kann ich nicht nachvollziehen, es geschieht doch die Zuweisung im Konstruktor

Die Warnung spricht von der initialization list.

EL europ

@Jockelx
Ich weiß nicht was genau die Initalisierungsliste bedeuted, aber diese Variable wird zur Laufzeit berechnet und wird public benötigt. Ich hoffe eure Compiler geben nur Hinweise aus das ich auf diese Variable ein Augenmerk haben soll, denn bei mir "meckert" er das nicht an

Finnegan

@EL-europ sagte in Klassenfrage :

Doch danke für deine Hinweise, da hab ich noch ne compilermeldung:
include/_Colorinterface.cpp:245:86: warning: narrowing conversion of ‘(((_Colorinterface*)this)->_Colorinterface::elements.std::vector<_CiElement>::size() - 1)’ from ‘std::vector<_CiElement>::size_type’ {aka ‘long unsigned int’} to ‘int’ [-Wnarrowing]

List Inizialization (Initialisierung von farbe2 in Zeile 245 in geschweiften Klammern) ist etwas penibler was die übergebenen Typen angeht. Diese Warnung bekommst du, weil du einen int mit einem Ausdruck vector.size() - 1 initialisieren willst, der den Typ vector<T>::size_type hat, was meistens ein uint64_t ist (64 Bit ohne Vorzeichen statt 32 Bit mit Vorzeichen, wie es für int meistens der Fall ist).

Um die Warnung wegzubekommen, kannst du z.B. den id-Member von _farbe* zu einem std::size_t machen oder du castest den Ausdruck explizit in einen int, wobei du sicherstellen solltest, dass dieser keine Werte außerhalb des Wertebereichs von int haben kann und dabei komische Sachen passieren können.

Wenn ich versuche das letzte Objekt im std::vector mit der array.end() "-Funktion" anzusprechen, compiliert er mir nicht.

Die end()-Iteratoren von Containern verweisen nicht auf das letzte Element, sondern auf ein Element dahinter. Ein Zugriff auf dieses nicht-existierende Element ist Undefined Behaviour.

Alternativen sind die Funktionen vector.back() oder vector.at(vector.size() - 1)/vector[vector.size() - 1], die eine Referenz auf das letzte Element zurückgeben (UB falls vector keine Elemente hat), vector.end() - 1 falls es sich um einen Bidirectional Iterator handelt (bei vector-Iteratoren der Fall) oder vector.begin() + vector.size() - 1 (was aber nur bei einem Random Access Itetrator funktionieren sollte, das ist aber bei vector ebenfalls gegeben).

* Namen die mit Unterstrich gefolgt von einem Kleinbuchstaben beginnen sind in C++ übrigens für die Implementation reserviert, also für den Compiler und die Standardbibliothek. Auch wenn das oft keine Probleme macht, ist es empfehlenswert, solche Namen zu vermeiden.

EL europ

@Finnegan
Danke, die compiler-Meldung hab ich mit einem (int) cast wegbekommen und die Bereichsüberschreitung ist leicht in den Griff zu bekommen. Das mach ich jetzt... und der Hinweis mit den Unterstrichen, danke das wird Berücksichtigung finden

EL europ

Ich hab zur Laufzeit manchmal Abbrüche mit der Meldung "malloc(): invalid size (unsorted)". Denke das kommt von den c-arrays?
Aber die thread Funktion bekomm ich in c++ ohne ein neues Buch nicht hin, und sie ist mein ganzer stolz (Und ich muss den ganzen Hirnschmalz noch mal investieren)

ist es nicht schlimm schon Gedachtes nochmals denken zu müssen weil man Details vergessen hat? Das Alter!

Schlangenmensch

@EL-europ sagte in Klassenfrage :

@Quiche-Lorraine
Da triffst du den Nagel zumindest nah am Kopf:
Ich allociere in einer Klasse c-arrays, wenn ich Objekte dieser Klasse in std::vector speichere und diese Arrays im Destruktor mit free() freigebe, funktionieren entsprechende Methoden der Objekte in std::vector nicht mehr. Ich geb zu das die c-arrays und c-threads "quick n dirty" Lösungen sind weil ich meine thread-Lösung nicht einfach in c++ threads umgesetz bekam, wie den Rest des Codes

Ich würde darauf wetten, dass die Klasse, die selbst Speicher alloziert kaputt ist und du die Rule of five nicht befolgst. Soll heißen, deine Klasse verhält sich anders, wenn Kopien von ihr angelegt werden, als du denkst.
Da ein Vektor mit wächst, kann es sein, dass die in ihm gespeicherten Objekte kopiert werden müssen.

Solange du keine GUI Library verwendest (gerade für GUIs gibt es da schonmal ausnahmen) würde ich darauf wetten, das du hervorragend ohne new auskommst.

Nutze kein newund kein malloc
Du brauchst ein Array -> nutze std::vector
Du meinst Pointer zu brauchen -> nutze std::unique_ptr oder std::shared_ptr(bevorzugt ersteres)
Nutze keine globalen Variablen. Wenn du ein Objekt der Klasse wo anders brauchst, übergeb es als Parameter.

Wenn du die ersten drei Punkte befolgst, brauchst du wahrscheinlich keinen selbstgeschriebenen Destruktor mehr. Außerdem funktionieren Sachen, wie Objekte kopieren einfach so von sich aus richtig, oder lassen sich nicht kopieren (wenn ein std::unique_ptr als Member verwendet wurde)
.

EL europ

@Schlangenmensch
Ich hab die Objekte der Klasse nicht mehr in std::vector, sonder "initialisiere" ein einziges Objekt mit anderen Parametern, und speichere die Parameter als struct in einem std::vector. Ich hoffte das die c-arrays damit laufen würden (ich vermute nur die c-arrays sind das prob)

Schlangenmensch

@EL-europ Der Einzige Unterschied zwischen class und struct in C++ ist die default Sichbarkeit der Member.
Ich vermute, dass dein Code einige Probleme haben wird, das zeigt einfach die Erfahrung wenn Leute von C mal schnell auf C++ wechseln (und insbesondere, wenn sie C schon nicht so richtig gelernt haben).

Wenn du in deinem struct kein new, malloc, delete oder free hast, ist die Chance gut, das das in dem Vektor funktioniert.

Aber, ob und wo du Probleme in deinem Code hast, oder haben könntest, wird man dir nur sagen können, wenn du ihn hier mit uns teilst.

EL europ

@Schlangenmensch
https://github.com/momefilo/mandelbrodt

Finnegan

@EL-europ sagte in Klassenfrage :

malloc(): invalid size (unsorted)

Das riecht sehr stark nach einer beschädigten Datenstruktur der malloc-Implementierung. Eventuell schreibt dein Programm in Speicherbereiche, wo malloc seine internen Daten verwaltet. Oder du hast irgendwo einen Pointer mit free freigegeben, der zuvor nicht mit malloc reserviert wurde. Z.B. kann mit malloc reservierter Speicher nur mit free freigegeben werden und solcher, der mit new reserviert wurde nur mit delete. Die Mechanismen sind nicht untereinander austauschbar.

Da du mit GCC kompilierst, würde ich dir mal empfehlen, dein Programm mal mitein paar aktivierten Sanitizern (-fsanitize=...) und Debug-Informationen (-g, für Infos zu Quellcode-Datei und Zeilennummern) zu bauen. Z.B. indem du folgede Parameter zu deinen gcc-Aufrufen im Makefile hinzufügst:

-g -fsanitize=address -fsanitize=leak -fsanitize=undefined

Clang kennt diese Sanitizer übrigens auch - sogar noch ein paar mehr, -fsanitize=memory (entdeckt Verwendung von nicht-initialisiertem Speicher) wird z.B. von GCC (noch) nicht unterstützt.

Mit diesen aktivierten Sanitizern steigt dein Programm dann aus, wenn ein Buffer Overflow, Use-After-Free, ein Speicherleck oder ein Undefined Behaviour-Konstrukt gefunden wird. Mit zusätzlichen Informationen wo im Code das Problem entdeckt wurde.

Die Sanitizer garantieren nicht, dass jedes Problem gefunden wird, aber sie finden schon eine ganze Menge und sind ein sehr nützliches Werkzeug, das direkt bei deinem Compiler dabei ist. Es lohnt sich, sich mal mit denen zu beschäftigen - es gibt auch noch andere nüzliche Sanitizer.

Ansonsten nimm dir auch die Empfehlungen von @Schlangenmensch und anderen hier zu Herzen. Wenn du ohne new und malloc auskommst oder zumindest Regeln wie die Rule of 3/Rule of 5 sorgfältig berücksichtigst, wirst du diese Art von Problemen wahrscheinlich gar nicht erst bekommen.

wob

@EL-europ sagte in Klassenfrage :

@Schlangenmensch
https://github.com/momefilo/mandelbrodt

Last commit von vor 17h.

Ich sehe im Toplevel cpp gerade das hier:

void newApple(){
		myApples.push_back(*new _AppleData);
		myApples.at(aktApple).xres = Apple->xres;
		myApples.at(aktApple).yres = Apple->yres;
		myApples.at(aktApple).depth = Apple->depth;
		myApples.at(aktApple).rmin = Apple->rmin;
		myApples.at(aktApple).rmax = Apple->rmax;
		myApples.at(aktApple).imin = Apple->imin;
		myApples.at(aktApple).imax = Apple->imax;
		myApples.at(aktApple).xpos = Apple->xpos;
		myApples.at(aktApple).ypos = Apple->ypos;
		myApples.at(aktApple).width = Apple->width;
		myApples.at(aktApple).height = Apple->height;
		aktApple++;
}

a) Du solltest keine globalen Variablen verwenden (weder myApples noch Apple).
b) Das ist doch einfach nur der Copy-Constructor. Was soll dieser Code?! Komplizierte Schreibweise für myApples.push_back(Apple)?

Und ich persönlich würde Variablen immer klein schreiben, Klassen/Structs groß. Dein Apple ist ja eine Variable. Der Typ ist _Apple*. Brr. Underscore & pointer weg! Und was soll diese Duplizierung von _Apple und _AppleData? (ah, ist vielleicht doch kein Copy-Constructor, sieht aber so aus, als sollte es einer sein, weil du 2 Klassen mit fast demselben Inhalt hast)

omggg

(Das Forum harkt gerade extrem bei mir...)

Man schaue auch mal nach den anderen Madelbrot-Themen in diesem Forum ... oder geht es darum gar nicht?

EL europ

@wob
diese struct _AppleData speichert nur die Parameter-Sätze mit denen das einzige Objekt der Klasse _Apple immer wieder neu initialisiert wird.
Es gibt vier Klassen von denen allen zur Laufzeit jeweils nur ein Objekt mit new instaziert wird.
Colorinterface->Apple->Userinterface->Display.
Ein einziges Objekt der Klasse "_Display" wird einem einzigen Objekt der Klasse " _Userinface" übergeben, dieses Userinterface-Objekt wird einem einzigen Objekt der Klasse _Apple übergeben welches slebst dem einzigen Objekt der Klasse "_Colorinterface" Übergeben wird.

Ich "liebe" diese Mandelbrotbilder und hab mit meiner ersten Python version diesen Programms und ffmpeg, Filme mittels Bildfolge der einzelnen "Farb-iterationsstufen" erstellt; Die sind Großformatfüllend betrachtet, berauschend.
Um solch eine Funktionalität nutzerfreundlich zu realisieren ist dieses mandelbrodt gedacht.
Ich habe Start- und Endparameter solch einer Bildsequenz vor meinem geistigen Auge, welche die Berechnung steuern. Muss aber alles erst noch Programmiert werden, bis jetzt ist ja nur möglich in das Apfelmänchen zu zoomen und es einzufärben und dann zu betrachten. (Von den spontanen Abstürzen abgesehen, die ich erst weg haben will bevor Funktionalität hinzugefügt wird)

EL europ

@omggg
Jawoll ich bekomm mit deinen flags nen haufen compilermeldungen mit denen ich mich nun auseinander setzten kann. Das -fsanitaze:memory funktioniert bei mir nicht, aber die Anderen werfent einen haufen Meldungen aus. Die versuch ich erst mal zu verstehen. Danke

EL europ

@Finnegan
Verstehe ich das richtig, das -fsanitize mir Meldungen(Abstürze) erst zur laufzeit "verursacht" ?
Ah blöde Frage, eben seh ich das er es gar nicht kompiliert mit diesem Flag

EL europ

Findet sich hier vielleicht ein Kenner in c++ der mir die einfachste Klasse "_Display" mit ihren drei kleinen Methoden in sauberem c++ darlegen kann, als minimal-Bsp-Code in Betracht ihrer Beziehung Colorinterface->Apple->userinterface->Display. Dann kann ich den Rest nach diesem Muster neu gestalten. Sonst muss ich durch den "Wolf" weil mir grad die Mittel fehlen für ein neues Fachbuch