ra3xdh / qucs_s

Qucs-S is a circuit simulation program with Qt-based GUI
https://ra3xdh.github.io/
GNU General Public License v2.0
844 stars 109 forks source link

Get rid of Q3PtrList wrapper #748

Open ra3xdh opened 3 months ago

ra3xdh commented 3 months ago

This is a long term refactoring task. It doesn't directly affect the simulation or program appearance, but will improve the code readability after implementation. Currently the main application uses a wrapper for Q3PtrList class in qt3_compat/q3ptrlist.h It would be good to replace this with QList.

TODO list

wawuwo commented 3 months ago

Getting rid of Q3PtrList has a subtle complication: some parts of the codebase use the "auto delete" feature.

qucs % grep -rF setAutoDelete | grep -vF qt3_compat
./schematic.cpp:    DocComps.setAutoDelete(true);
./schematic.cpp:    DocWires.setAutoDelete(true);
./schematic.cpp:    DocNodes.setAutoDelete(true);
./schematic.cpp:    DocDiags.setAutoDelete(true);
./schematic.cpp:    DocPaints.setAutoDelete(true);
./schematic.cpp:    SymbolPaints.setAutoDelete(true);
./schematic.cpp:    Wires->setAutoDelete(false);
./schematic.cpp:    Components->setAutoDelete(false);
./schematic.cpp:    Wires->setAutoDelete(true);
./schematic.cpp:    Components->setAutoDelete(true);
./schematic.cpp:    Wires->setAutoDelete(false);
./schematic.cpp:    Components->setAutoDelete(false);
./schematic.cpp:    Wires->setAutoDelete(true);
./schematic.cpp:    Components->setAutoDelete(true);
./schematic.cpp:    Wires->setAutoDelete(false);
./schematic.cpp:    Components->setAutoDelete(false);
./schematic.cpp:    Wires->setAutoDelete(true);
./schematic.cpp:    Components->setAutoDelete(true);
./schematic.cpp:    Components->setAutoDelete(false);
./schematic.cpp:    Components->setAutoDelete(true);
./schematic.cpp:    Wires->setAutoDelete(false);
./schematic.cpp:    Wires->setAutoDelete(true);
./mouseactions.cpp:        Doc->Wires->setAutoDelete(false);
./mouseactions.cpp:        Doc->Wires->setAutoDelete(true);
./diagrams/diagramdialog.cpp:  Graphs.setAutoDelete(true);
./diagrams/diagramdialog.cpp:  Graphs.setAutoDelete(false);
./diagrams/diagramdialog.cpp:  Graphs.setAutoDelete(true);
./schematic_element.cpp:    Wires->setAutoDelete(false);
./schematic_element.cpp:    Components->setAutoDelete(false);
./schematic_element.cpp:    Wires->setAutoDelete(true);
./schematic_element.cpp:    Components->setAutoDelete(true);
./components/libcomp.cpp:    pc->Props.setAutoDelete(false);
./components/component.cpp:    Props.setAutoDelete(true);
./components/component.cpp:        Doc->Components->setAutoDelete(false);
./components/component.cpp:        Doc->Components->setAutoDelete(true);

If it were only for ownership control that would be a bit easier to refactor, but as I understand some functional logic relies on it too, which makes it impossible in some cases to straightforwardly replace one with another.

ra3xdh commented 3 months ago

Understood. Then let's keep the things as is. Let this issue be open for some time. Maybe some solution will appear.

dsm commented 2 months ago

https://github.com/Qucs/qucs/commit/141b840ed19c5a98de922f06cff7f99261bc9ed4 qt3_combat removed.

ra3xdh commented 2 months ago

https://github.com/Qucs/qucs/commit/141b840ed19c5a98de922f06cff7f99261bc9ed4 qt3_combat removed.

Yes, we could learn how this patch is done and use the similar concept for Qucs-S. At least Component::Properties could be migrated to QList. I had a quick look at this patch and I didn't like mixing of QList and std::list. But the similar concept could be used. Reopening this.

ra3xdh commented 2 months ago

I believe the sharedObjectList class proposed by the patch mentioned above could be replacement for Q3PtrList and resolve the problem with setAutoDelete. I am not sure if this container will be recognized by GDB debugger.

dsm commented 2 months ago

I looked a bit refactoring some section but some code used Linklist some section use List so we should be mixed std::list and QList or QLinkList in qt5combat module or std::vector maybe used because some section use at() method.

ra3xdh commented 2 months ago

Reopening. Not finished yet.

dsm commented 2 months ago

QList has stl-style or java style iterator some code section call next hasNext maybe those 2 class can be use.

https://doc.qt.io/qt-6/qlistiterator.html https://doc.qt.io/qt-6/qmutablelistiterator.html

ra3xdh commented 1 month ago

I have added a TODO list for this issue. Maybe the solution for Schematic class members will appear later. Let's keep this issue open.