topic deleted
-
Dieser Beitrag wurde gelöscht!
-
Warum hast du z.B. in Cell den Copy Constructor und Zuweisungsoperator implementiert? Ist es nicht genau das, was auch der Compiler gemacht hätte? In dem Fall finde ich das völlig kontraproduktiv.
Warum hat die GraphicsCell ein SettingsWindow? Das hört sich nicht nach einer Klasse an, die irgendwas über irgendwelche Windows wissen sollte. Allerdings greifst du da nur auf irgendwelche Einstellungen zu. Evtl. solltest du deinen Code anders strukturieren und eine Settings (ohne Window) Klasse haben, dann würds auch nicht negativ ins Auge springen.
Seperate?
Der Thread sollte auch kein GraphicWindow oder so kennen, ist auch eine Klasse, die mit den Domänenobjekten, nicht mit der GUI (ja, mir gefällt die GUI auch besser als der, aber der wär wohl richtig) arbeiten sollte.
Die SettingsWindow Klasse schaut übel aus. Kannst du das nicht mit einer ui Datei erstellen? Kein Mensch will hunderte Zeilen Boilerplate Code lesen, der nur GUI "zusammenklickt". Wenn die GUI komplex wäre, klar. Aber danach schauts mir nicht aus.
if(GraphicCell::clicked) gefällt mir nicht.
Ob das Rumhantieren mit der QGraphicsScene/View richtig ist, kann ich grad nicht sagen, so genau will ich mir grad nicht reindenken
Mir kommts aber so vor, dass irgendwelche Überprüfungen, ob du innerhalb der Scene-Grenzen bist, nicht nötig sind, das könnte man wahrscheinlich anders machen. z.B. gibts Signale, wenn ein QGraphicsItem angeklickt wurde, und dann weißt du auch gleich, welches das war.
-
Dieser Beitrag wurde gelöscht!
-
Skylac06 schrieb:
Dadurch wurden natürlich copy-ctor und operator=() überflüssig. Ich werde es ausbessern.
Ich versuche, sowas grundsätzlich zu vermeiden, ist nur eine zusätzliche Fehlerquelle. Und es gibt ganz selten Fälle, wo man einen eigens definierte Copy Constructor braucht.
Skylac06 schrieb:
Meinst du, dass die Funktionen saveGame() und loadGame() dann auch nicht in die Klasse passen? Diese gehören eigentlich am ehesten in GraphicalWindow. Oder wie siehst du das?
Ja, eher nicht in die Settings. Je nachdem, wie "groß" das ganze wird, würds hier wahrscheinlich Sinn machen, eine eigene Game Klasse zu machen, die load und save kann. Und der Code drum, mit dem Aufmachen des Open File Dialogs gehört für mich eher in eine "Haupt-Fenster" Klasse. Also, ein Fenster darf quasi die Logik steuern: Dialog anzeigen, Game Klasse laden, an die GraphicsScene/View übergeben. Das GraphicalWindow hätt ich vielleicht eher von QGraphicsView abgeleitet und entsprechend vielleicht GameOfLifeView oder so genannt. Und da kommen solche Sachen rein, wie das Handling der Mausevents usw., aber nicht unbeingt anzeigen von Dialogen.
Was du wegen den Threads schreibst, hab ich nicht 100% verstanden. Die GUI darfst du jedenfalls nur im GUI Threads aktualisieren. Aber du kannst von deinem Thread ein Signal emitieren, und das kannst mit einer QueuedConection verbinden, dann wird der Slot im richtigen Thread aufgerufen.
Ich hab in der Arbeit praktisch nie mit ui Dateien zu tun. Allerdings haben wir fast ausschließlich "komplexe" GUI, die komplett eigens gezeichnet wird (meist TreeViews mit eigenen Delegates + Laden der Daten im Hintergrund, Caching usw.). In letzter Zeit sind so gut wie keine einfachen Dialoge mit irgendwelchen Eingabefeldern hinzugekommen. Allerdings würde ich die dann schon mit dem QtDesigner erstellen. Dieser "GUI Code" ist völlig uninteressant, wenn du damit etwas Erfahrung hast, wirst du das alles weder schreiben noch lesen wollen.
Skylac06 schrieb:
Das Problem ist, dass man das mouseMoveEvent() nicht QGraphicsItem-übergreifend verwenden kann. Die können untereinander nicht kommunizieren. ...
Kann sein, ja. Ist schon paar Jahre her, dass ich was komplexeres mit QGraphicsScene gemacht habe, und teilweise habe ich da auch länger gebraucht, um auszutüfteln, wie das alles funktioniert. Deswegen kann ich dir grad auch nicht sagen, inwiefern dein Code hier optimal ist, dafür müsste ich mich wieder tiefer einarbeiten. Ich hatte nur den Eindruck, dass es besser funktioniert, wenn man mal verstanden hat, wie das Framework gedacht war, und sich darauf einlässt. Ist nur ein Hinweis, ob das bei dir der Fall ist, weiß ich nicht.
-
Dieser Beitrag wurde gelöscht!
-
Skylac06 schrieb:
Eigentlich jedes Mal, wenn man dynamisch Speicher holt. Das versucht man natürlich zu vermeiden, aber manchmal geht es eben nicht anders. Und gerade mit Qt braucht man das doch recht häufig. So selten aber auch nicht.
Keine rohen, besitzenden Zeiger verwenden. Entweder smart pointer, oder wenn man Qt nutzt dann die Speicherverwaltung über die Parent Beziehung regeln.
Das system könntest du z.B. in einen reference_wrapper stecken.
-
Dieser Beitrag wurde gelöscht!
-
Skylac06 schrieb:
Ist vielleicht aber tatsächlich dann schlechter Stil. Sollte ich das ändern?
Ja, eher schon. Besitzende rohe Zeiger verursachen tendentiell zu viele Probleme, deswegen lohnt sich die Überlegung eigentlich nicht wirklich.
Warum bool und int nicht einfach per value zurückgeben? Find ich jetzt nicht schlimm, seh da aber auch keinen Vorteil, die als const& zurückzugeben.
Das Rumhantieren mit strings schaut komisch aus. Ich bin selber auch kein großer Fan der Qt und mir ist die STL lieber. Aber du wandelst (in loadGame) erst einen QString in einen std::string, dann holst dir c_str und dann machst daraus wieder einen QString. Dann bleibt doch einfach bei dem QString.
Das readSaveFile ist auch so ein bisschen ein Anti-Pattern. Lies mal die C++ Core Guidelines. Bei einem Zeiger Parameter sollte man per default davon ausgehen, dass damit keine Ownership übertragen wird. Etwas außerhalb der Funktion zu erstellen und innerhalb einer Funktion zu zerstören, ist keine gute Idee. Außerdem willst du eigentlich auch keinen Zeiger, weil du nicht mal prüfst, ob er gültig ist, sondern erwartest, dass er gültig ist. Das spricht erst recht für eine Referenz. Und du müsstest QFile nicht mal auf dem Heap erstellen, hättest die auch auf dem Stack erstellen können.
Separate, nicht seperate
Friend an sich ist noch kein Problem. Aber dass Settings das Settingswindow kennt, gefällt mir nicht. Abhängigkeiten sollten in eine Richtung gehen, nicht in beide. Wär besser, wenn Settings eine reine Datenverwaltungsklasse wäre, und nur SettingsWindow das kennt.
-
Dieser Beitrag wurde gelöscht!
-
Skylac06 schrieb:
Ich hätte gerne nur QString in dem Fall genommen - oder auch nur std::string.
Aber QString hat weder die Methode std::string::substr() noch std::string::find_last_of().Scheinbar hast du dir nicht die Doku zu QString angeschaut.
http://doc.qt.io/qt-5/qstring.htmlfür string::find_last_of gibt es eine direkte entsprechung auch in QString QString::lastIndexOf
für std::string::substr ist es QString::mid
-
Dieser Beitrag wurde gelöscht!