Making KFileMetaData a framework
David Edmundson
david at davidedmundson.co.uk
Tue Aug 5 17:58:03 UTC 2014
In general that's some of the tidied code I've seen in a long time.
Comments below. One major, most not.
----
TypeInfo/PropertyInfo/SimpleExtractionResult need a working &operator=
otherwise we shallow copy d.
I can cause a crash in dump.cpp
----
Extracting info from a file can take a long time, right now you spawn
a separate execuatble for Baloo; but for other uses (i.e dolphin
getting info on a non-indexed file) you might want to add a helper
API; either a new thread or a service like baloo-file-extractor and a
wrapper round calling it.
----
ExtractorPluginManager::Private::allExtractors
what's going on with the variable "plugins"? You're always checking if
an empty list contains things. Looks leftover from something.
----
In properties.h I would space out the enum so that in the future when
you insert extra ones you can put it alongside the relevant category
without breaking API
i.e
//Audio
BitRate = 100,
Channels,
Duration,
// Documents
Author =200,
Title,
...
Otherwise it'll be an ungrouped mess in future releases if you ever
have to add another audio property.
-----
ExtractionPluginManager possibly shouldn't be a QObject?
It /might/ be a good idea to make ExtractionPluginManager static so
you don't load the plugins every time (which can be a bit expensive)
----
dump.cpp should check there's at least one arg.
(I know it's a test, but you explicitly check for >=2 but not <1)
----
More information about the Kde-frameworks-devel
mailing list