https://bugs.kde.org/show_bug.cgi?id=374225--- Comment #17 from Mario Frank <
[hidden email]> ---
(In reply to Simon from comment #16)
> > --- Comment #15 from Mario Frank <
[hidden email]> ---
> > Simon, polishing my patches is completely okay.
> > Just to be safe. Deleting the region, the identity and the connection from tag
> > to image is okay?
> I think deleting the region and identity by default, and asking about
> the tag is a sane behaviour.
Okay.
> > Should I sync the metadata or not? What should I adopt in my patch?
> >
> Yes, metadata should be synced (always). However this is not done in
> many (if not all, I didn't check) methods of tagmodificationhelper. The
> other methods delete tags via AlbumManager and looking at the right tag
> sidebar, syncing metadata involves some fileworker interfaces. So this
> seems quite a big and separate fix that is not directly related to this.
> I discuss/solve that part in a new issue.
Okay, I will look how to sync the metadata.
>
> Do you agree on Gilles proposition to change "Delete person tag" to
> "Remove Face Tag"?
Yes, I agree. I will adopt that.
>
> Another question: The patch introduced a new method with an sql query.
> In the existing code such queries are sometimes done directly via
> execSql, at other places via get-/exec- DBAction, so from
> dbconfig.xml.cmake.in .
> What decides whether a query gets into dbconfig.xml.cmake.in or whether
> it doesn't?
I cannot give a statement to this one.
>
> And only for my education, so ignore at will:
> In the tagmodificationhelper methods, arguments are sometimes passed by
> reference (lists), sometimes not (single album) while both underlying
> types are pointers. Any particular reason for that?
That's not easy to explain since there may be many reasons. Some are technical,
some
ideological.
The difference is subtile. While complex (big) structures like single albums
are handled in heap a list of
X (e.g. pointers) is usually located in the stack.
Consider that.
A list of pointers with each pointer being 64 bit aka 8 byte does not take much
space.
Even if you store 100000, it's only 800000 byte aka 800 kilobyte. that's okay.
But passing a list of this size by copy is expensive. Thus, you need a
reference, to
reduce the overhead. Also, you can modify the list in the called function.
Whether you take a pointer or a ref does not make a difference (both collapse
to 8 byte).
But refs cannot be null, while pointers can. And if you know that you have an
at least empty list,
pointers do not make sense here.
Passing a const pointer only forbids the manipulation of the pointer, but not
the contents of the object.
Passing a const ref list forbids altering the list. This is far more secure
than passing a const pointer.
Also, using pointers may indicate for other developers that the passed object
is located in heap which may not
be always the case. The developer could try to deallocate the object stored in
stack which will impose errors.
Though I do not think that this would compile.
--
You are receiving this mail because:
You are the assignee for the bug.
Locked
