michaelh added inline comments.
INLINE COMMENTS
> svuorela wrote in documentid.cpp:67
> if you make operator DeviceIdAndInode explicit, you probably end up with a
> bit better behavior.
>
> Else the compiler happily converts a documentid to a deviceidandinode in
> order to make various comparisons at compile time.
>
> e.g. DocumentId id(5,5);
> bool b = true;
> if (id == b) { ... } should compile, but probably isn't intended behavior..
This is intended. At the moment the class should behave like
quint64(=DeviceIdAndInode)
> svuorela wrote in documentid.h:51-52
> it depends on why it is exported. If it is exported just for unit test (and
> the header file not installed), then we don't need to have the mental and
> code wise overhead of a d-pointer.
>
> If we know that it will never ever need to grow, we also don't need the
> overhead of a d-pointer.
> So it is a big "it depends", which I was also kind of trying to ask into
> earlier.
I have to admit, I have not understood the meaning of BALOO_ENGINE_EXPORT yet.
The file which defines it looks like it's automatically created. I inserted it
after trial and error. Except for testing I see no reason why it should be
exported. I consider it as strictly internal.
In its present state this class shall serve as a replacement for the quint64
that baloo is currently using for an id.
Finally this class will have a payload of 24 bytes max. (64bit inode and 128bit
uuid) and one method: `isValid()`. That at least is the plan.
This class is used a lot. What I have read about d-pointers so far indicates a
huge performance hit when using indirection. Maybe I'm wrong.
@svuorela: I'm sorry if my answer appeared uncouth. That was not intended.
Currently a lot of my programming is the result of trial and error rather than
knowledge. In time the ratio, hopefully, will change in favor of the latter :)
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D10826
To: michaelh, adridg, #baloo, #frameworks
Cc: anthonyfieroni, svuorela, ashaposhnikov, michaelh, spoorun, nicolasfella,
alexeymin
