Kurdder
Anmeldungsdatum: 12. April 2009
Beiträge: 85
Wohnort: P-Town (RLP)
|
Hallo Freunde, habe mein erstes eigenes Projekt fertig und woltle es mal hier vorstellen: Qmpare ist ein Programm, um mehrere Dateien auf einen bestimmten String hin zu vergleichen. Man wählt einen Ordner aus und die darin enthaltenen Dateien, sowie der Pfadname können rekursiv in einer Tabelle dargestellt werden. Hier kann man nun einzelne Objekte auswählen und über ein "+"Icon zur sogenannten Qmpareliste hinzufügen. Mit einem klick auf ::qmpare:: werden dann die aufgelisteten Dateien auf den Stringinhalt überprüft, den man im dafür vorgesehenem Eingabefeld hinterlassen hat. Geschrieben in C++ mit Hilfe des Qt-Framework.
Hoffe auf Verbesesrungsvorschläg (bspw die Gestaltung oder optimierung des Codes), vllt kann/will auch der ein oder andere was dazu beitragen(evtl, eine Multiselektion o.Ä.). https://sourceforge.net/projects/qmpare/ P.S.: Momentan zeigt das Programm nur die letzte Datei an, welche den String enthält und nicht alle Dateien..
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
Ich habe deinen Source mal überflogen und ich glaube, mein erstes Programm war nicht so übersichtlich gestaltet. 😀 Allerdings eine Sache ist mir dann doch ins Auge gestochen, wieso verwendest du keine lokalen Variablen in den Funktionen, sondern erzeugst fast überall neue Objekte im Heap? Dadurch umgehst du die Garbage Collection in C++, was zu Memory Leaks führen kann und musst dich zudem selber um die Löschung der Objekte kümmern.
|
Kurdder
(Themenstarter)
Anmeldungsdatum: 12. April 2009
Beiträge: 85
Wohnort: P-Town (RLP)
|
Ist ja nicht mein erstes Programm..hab ein paar Projekte/Programm verworfen weil sie einfach zu unübersichtlich wurden nach einiger Zeit 😛 Da hat man halt dazu gelernt^^ Beim rekursiven Suchdurchlauf erzeug ich über new/delete kein Segmentation Fault. Daher ist es zumindest an dieser Stelle von Nöten.
Ist doch auch weniger Speicherintensiv und schneller im Programmfluß oder irre ich mich?
|
domachine
Anmeldungsdatum: 16. Mai 2007
Beiträge: 562
|
Vegeta schrieb: Ich habe deinen Source mal überflogen und ich glaube, mein erstes Programm war nicht so übersichtlich gestaltet. 😀 Allerdings eine Sache ist mir dann doch ins Auge gestochen, wieso verwendest du keine lokalen Variablen in den Funktionen, sondern erzeugst fast überall neue Objekte im Heap? Dadurch umgehst du die Garbage Collection in C++, was zu Memory Leaks führen kann und musst dich zudem selber um die Löschung der Objekte kümmern.
In C++ gibt es streng genomment keine "Garbage Collection". Objekte, die nach dem von dir genannten Schema angelegt werden, werden lediglich auf den Stack gelegt. Deshalb sind sie beim Austritt aus entsprechenden Blöcken automatisch wieder verschwunden, weil sie zusammen mit dem Funktionsaufruf wieder vom Stack entfernt werden. Außerdem ist es in Qt Gang und Gebe Objekte dynamisch zu alloziieren. Qt erwartet das an diversen Stellen sogar, weil es sich aufgrund der QObject s praktisch selber um die Speicherverwaltung kümmert und Objekte in Abhängigkeit zerstört (Es könnten immer noch Signal-Verbindungen zwischen Objekten bestehen). Somit ist es viel eleganter, Objekte dynamisch zu alloziieren und als "Kind"-Objekt einer Oberklasse zu deklarieren und diese Oberklasse per deleteLater() automatisch bei der Rückkehr zur "Event"-Schleife zerstören zu lassen. Ich habe mir nicht die Quellen angesehen, weshalb ich auch nicht direkt darauf eingehe, ob Kurdder das von mir benannte hier richtig umsetzt. Mir sind nur deine (Vegeta) Äußerungen aufgefallen, die ich für verbesserungswürdig errachtete. Gruß Dominik
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
Kurdder schrieb: Beim rekursiven Suchdurchlauf erzeug ich über new/delete kein Segmentation Fault. Daher ist es zumindest an dieser Stelle von Nöten.
Wenn dem so ist, dann ist an der Stelle ein Fehler. Ich würde dir ohnehin empfehlen QDirIterator zum Einlesen der Verzeichnisse zu benutzen.
Ist doch auch weniger Speicherintensiv und schneller im Programmfluß oder irre ich mich?
Wieso sollte es weniger speicherintensiv sein? Von der Geschwindigkeit keine Ahnung, bei der geringen Masse an Daten aber ohnehin nicht feststellbar. Es macht den Source aber auf jeden Fall unübersichtlicher, wenn die Objekte überall manuell gelöscht werden müssen. Genau für diesen Zweck sind lokale Variablen da, dass sie nur in der jeweiligen Funktion/Abschnitt existieren und danach sofort automatisch gelöscht werden. Aber muss jeder selber wissen wie er es macht, falsch ist es jedenfalls nicht. 😉
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
domachine schrieb: In C++ gibt es streng genomment keine "Garbage Collection". Objekte, die nach dem von dir genannten Schema angelegt werden, werden lediglich auf den Stack gelegt. Deshalb sind sie beim Austritt aus entsprechenden Blöcken automatisch wieder verschwunden, weil sie zusammen mit dem Funktionsaufruf wieder vom Stack entfernt werden.
Richtig Objekte im Stack sind flüchtig, alles was im Heap angelegt wird überdauert und das führt bei unachtsamer Verwendung durchaus zu Memory Leaks.
Außerdem ist es in Qt Gang und Gebe Objekte dynamisch zu alloziieren. Qt erwartet das an diversen Stellen sogar, weil es sich aufgrund der QObject s praktisch selber um die Speicherverwaltung kümmert und Objekte in Abhängigkeit zerstört (Es könnten immer noch Signal-Verbindungen zwischen Objekten bestehen).
Es ist auch in Qt möglich komplett ohne Pointer zu arbeiten, auch wenn diverse Stellen, allen voran die GUI, dieses explizit verlangen.
|
domachine
Anmeldungsdatum: 16. Mai 2007
Beiträge: 562
|
Vegeta schrieb: domachine schrieb: In C++ gibt es streng genomment keine "Garbage Collection". Objekte, die nach dem von dir genannten Schema angelegt werden, werden lediglich auf den Stack gelegt. Deshalb sind sie beim Austritt aus entsprechenden Blöcken automatisch wieder verschwunden, weil sie zusammen mit dem Funktionsaufruf wieder vom Stack entfernt werden.
Richtig Objekte im Stack sind flüchtig, alles was im Heap angelegt wird überdauert und das führt bei unachtsamer Verwendung durchaus zu Memory Leaks.
Außerdem ist es in Qt Gang und Gebe Objekte dynamisch zu alloziieren. Qt erwartet das an diversen Stellen sogar, weil es sich aufgrund der QObject s praktisch selber um die Speicherverwaltung kümmert und Objekte in Abhängigkeit zerstört (Es könnten immer noch Signal-Verbindungen zwischen Objekten bestehen).
Es ist auch in Qt möglich komplett ohne Pointer zu arbeiten, auch wenn diverse Stellen, allen voran die GUI, dieses explizit verlangen.
Die Frage war nicht, ob es geht, sondern was von Qt preferiert (und somit eleganter ist) wird. Und das sind eindeutig die dynamisch alloziierten Objekte die von Qt selbst wieder freigegeben werden. Ich hoffe da stimmst du mir zu. Ich würde sagen wenn schon Qt, dann auch richtig.
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
domachine schrieb: Die Frage war nicht, ob es geht, sondern was von Qt preferiert (und somit eleganter ist) wird. Und das sind eindeutig die dynamisch alloziierten Objekte die von Qt selbst wieder freigegeben werden. Ich hoffe da stimmst du mir zu. Ich würde sagen wenn schon Qt, dann auch richtig.
Generell ja, trotzdem kann es sinnvolle Ausnahmen geben. Qt löscht aber nicht überall alle alloziierten Objekte, teilweise muss man entweder mit besagten delete, deleteLater oder per connect sie selber löschen.
|
domachine
Anmeldungsdatum: 16. Mai 2007
Beiträge: 562
|
Vegeta schrieb: domachine schrieb: Die Frage war nicht, ob es geht, sondern was von Qt preferiert (und somit eleganter ist) wird. Und das sind eindeutig die dynamisch alloziierten Objekte die von Qt selbst wieder freigegeben werden. Ich hoffe da stimmst du mir zu. Ich würde sagen wenn schon Qt, dann auch richtig.
Generell ja, trotzdem kann es sinnvolle Ausnahmen geben. Qt löscht aber nicht alle alloziierten Objekte, teilweise muss man entweder mit besagten delete, deleteLater oder per connect sie selber löschen.
delete ist bei Qt fast immer eine schlechte Idee.
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
domachine schrieb: delete ist bei Qt fast immer eine schlechte Idee.
QTreeWidgetItem ist so eine Stelle wo es Sinn macht oder man muss takeChild benutzen. Ist aber richtig, sollte man vermeiden.
|
domachine
Anmeldungsdatum: 16. Mai 2007
Beiträge: 562
|
Vegeta schrieb: domachine schrieb: delete ist bei Qt fast immer eine schlechte Idee.
QTreeWidgetItem ist so eine Stelle wo es Sinn macht oder man muss takeChild benutzen. Ist aber richtig, sollte man vermeiden.
Schön, dass wir zum selben Schluss kommen.
|
Vegeta
Anmeldungsdatum: 29. April 2006
Beiträge: 7943
|
domachine schrieb: Schön, dass wir zum selben Schluss kommen.
Türlich, die Alternative sind zum Teil immer wieder abstürzende Programme.
|
domachine
Anmeldungsdatum: 16. Mai 2007
Beiträge: 562
|
Nun bin ich auch mal dazu gekommen, den Code zu betrachten. Ich muss sagen an diversen Stellen sehr abstrus. Bedarf auf jeden Fall einiger Überarbeitung außerdem muss ich mich diverser Kritik von Vegeta anschließen.
Stellen wie folgende sind total aufgeblasen und enthalten eine Menge Kritikpunkte:
| QString *stringadd = new QString(item1->text().toAscii()+"/"+item2->text().toAscii());
|
Zunächst einmal ist hier die dynamische Alloziierung tatsächlich völlig unangebracht. Sie macht die Sache nur unnötig unkompliziert und ist auch gar nicht im Sinne der QString Klasse. Die ist nämlich dazu da um die veralteten C-Strings zu bedecken. Außerdem: Was zum Henker willst mit dem toAscii() -Einsatz erreichen? Ich kann keinen Sinn erkennen. Die QString Klasse hat bereits einen überladenen + Operator der die bequeme Konkatenation zweier Strings ermöglicht. Und wenn du schon anfängst mit Laufzeit zu argumentieren, kann ich dir versichern, dass du die mit solchen Aktionen keinesfalls verbesserst. Und darf man fragen wieso in deiner Suchfunktion ohne die "new-delete-Katastrophe" ein Segfault erzeugt werden soll? Hier ist besagtes nämlich genauso unangebracht. Die Objekte würden ohne new bequem beim Verlassen des Blocks zerstört und QString ist weiß Gott keine Klassen mit oben genannter Problematik. Der Code enthält noch weitaus mehr Sprengstoff. Überarbeite ihn nochmals unter den genannten Gesichtspunkten.
|
Kurdder
(Themenstarter)
Anmeldungsdatum: 12. April 2009
Beiträge: 85
Wohnort: P-Town (RLP)
|
Habe jetzt nochmal eine Neuauflage gemacht..
Hab die Tabelle verworfen und stattdessen mit QDirModel und QTreeView gearbeitet, Was die Ansicht/Auswahl erheblich verbessert hat. Und auch im Coding wesentlich einfacher war 😛 @domachine
Den SegFault hab ich, wenn ich bei der Parameterübergabe der Suchfunktion ohne den Dereferenzierungsoperator(*) arbeite.. Aber auch erst, sobald es um tiefe Ordnerstrukturen geht(Also bspw auf '/' angewendet)..immerhin erzeug ich dadurch zig male "curDir" auf dem Stack..Das kann doch nur dazu führen oder?!
Habe mich selbst in meinem vorangegangenem Post vertan und falsch ausgedrückt! Tut mir leid.
Ansonsten verstehe ich deine Verbesserungsvorschläge und werde das berücksichtigen ☺ Neu hinzugekommen
→ Verbesserte Ordneransicht/Datenstruktur durch QDirModel und QTreeView
→ Protokollierung
→ Tabs zum wechsel zwischen Ordnerstruktur und Protokoll
|