Hey Ben,
Thanks for sharing your post! I've gone over a little of it to try and give
you the feedback you asked for. Let me know if you want me to expand on
anything..
The first thing that stands out the most is that there is potential memory
leak and design flaw in your CustomProxyModel.
You have a public vector of "filters", which gets allocated on the heap by
your proxy, but your proxy class does not have a destructor and is not set
up to manage that memory. So the class on its own leaks memory. But then if
we look at the implementation of Multi_Array_Table class, we see that it
contains logic for "stealing" the widgets from the vector of the proxy and
taking ownership of them. On subsequent calls to that setup function it
will delete the previous widgets and clear the vector. So this combination
has a number of implications:
* The proxy model has a public setupFilters() method which will always
create and push more widgets into a vector each time it is called.
* The proxy model has no idea when some or all of the memory will be
deleted. If the Multi_Array_Table gets deleted and the proxy model is still
around, being used by another model, it will have dangling pointers in its
vector.
* The Multi_Array_Table class is relying on "stealing" ownership of widgets
created by another class while that other class still maintains a public
vector of the memory.
I don't feel a proxy model should be creating widgets. And it usually isn't
appropriate in C++ to have public access to fields like that, since any
caller can now have full access to modifying that vector. At the very least
you should probably add destructors to your classes, and maybe make use of
QPointer or QSharedPointer for tracking ownership in a more realiable way.
It would probably be even a better step to not have your proxy rely on
widgets, rather just pure data. If the view has the QLineEdits and can
update the proxy with the new text data when they change, then your proxy
can test QStrings instead of QLineEdits.
Other notes:
I don't often see the pattern of passing &Class::someSignal and
&Class:someSlot when calling connect().
Is this something you have seen in other examples? The usual documented way
involved SIGNAL("someSignal()")
and SLOT("someSlot()").
In your CustomProxyModel::filterAcceptsRow, you have it iterating and
building bitwise values. Would it not be simpler
to just return false when the string does not contain the regex, and then
return true at the end?
In CustomProxyModel::setFilter, you are identifying the target filter by
objectName, which seems a bit fragile. Wouldn't
just comparing the sender to the filterEdit work, since comparing the
pointers would tell you that they are the same?
Boost seems like a huge hammer to introduce as a dependency here, just for
using the recursive directory support in boost::filesystem.
Could you use QDirIterator <http://doc.qt.io/qt-4.8/qdiriterator.html> for
the same purpose and ditch boost?
There sure are a lot of "util" files in the project. Some of which do
really trivial things, such as just returning the string::c_str()
from a string. Others do some pretty inefficient work, like converting to
one kind of vector, then iterating that vector to convert
to another kind of vector. They seem to obscure the top level calls and
hide the costs. An example is convertToQStringList(), which
really only needs to do something like:
QStringList slist;
slist.reserve(vec.size()); // optional
foreach (const std::string &s, vec) {
slist << QString::fromStdString(s);
}
Justin
On Sun, Apr 9, 2017 at 7:18 AM Benjam901 <[email protected]> wrote:
Hello all,
I was wondering if anyone could do me a favour and have a quick glance at
my newest blog post about learning Model/View programming and the code
itself.
I am looking for some feedback on potential issues that could arise from my
programming, memory leaks, things that will go wrong in the future if not
taken care of now etc.
https://www.benhearntechartist.com/single-post/2017/04/08/Lightweight-C-Dynamic-ModelView-QSortFilterProxyModel
Cheers,
Ben
--
You received this message because you are subscribed to the Google Groups
"Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/python_inside_maya/f1f90e5d-18a2-44a5-94bc-66be89ea16c9%40googlegroups.com
<https://groups.google.com/d/msgid/python_inside_maya/f1f90e5d-18a2-44a5-94bc-66be89ea16c9%40googlegroups.com?utm_medium=email&utm_source=footer>
.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups
"Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA2dF6ftW46BOLdJuYeOqLJ4Q30Kk0YWmLkF%3D1zNwS_76w%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
