Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of Q3PtrList wrapper #748

Open
3 of 4 tasks
ra3xdh opened this issue Jun 11, 2024 · 9 comments · Fixed by #845, #854, #858 or #881
Open
3 of 4 tasks

Get rid of Q3PtrList wrapper #748

ra3xdh opened this issue Jun 11, 2024 · 9 comments · Fixed by #845, #854, #858 or #881
Assignees
Milestone

Comments

@ra3xdh
Copy link
Owner

ra3xdh commented Jun 11, 2024

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
Copy link
Contributor

wawuwo commented Jun 15, 2024

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
Copy link
Owner Author

ra3xdh commented Jun 15, 2024

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

@ra3xdh ra3xdh closed this as completed Jun 18, 2024
@dsm
Copy link
Contributor

dsm commented Jul 3, 2024

Qucs/qucs@141b840 qt3_combat removed.

@ra3xdh
Copy link
Owner Author

ra3xdh commented Jul 4, 2024

Qucs/qucs@141b840 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 ra3xdh reopened this Jul 4, 2024
@ra3xdh
Copy link
Owner Author

ra3xdh commented Jul 7, 2024

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
Copy link
Contributor

dsm commented Jul 9, 2024

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 ra3xdh linked a pull request Jul 9, 2024 that will close this issue
@ra3xdh ra3xdh added this to the 24.4.0 milestone Jul 9, 2024
@wawuwo wawuwo mentioned this issue Jul 16, 2024
@ra3xdh ra3xdh linked a pull request Jul 16, 2024 that will close this issue
@ra3xdh ra3xdh linked a pull request Jul 22, 2024 that will close this issue
@ra3xdh ra3xdh removed a link to a pull request Jul 22, 2024
@ra3xdh
Copy link
Owner Author

ra3xdh commented Jul 26, 2024

Reopening. Not finished yet.

@dsm
Copy link
Contributor

dsm commented Jul 30, 2024

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 ra3xdh self-assigned this Aug 1, 2024
@ra3xdh ra3xdh linked a pull request Aug 2, 2024 that will close this issue
@ra3xdh ra3xdh linked a pull request Aug 2, 2024 that will close this issue
@ra3xdh
Copy link
Owner Author

ra3xdh commented Aug 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment