On 2019-06-06 14:04, Lars Knoll wrote:
On 6 Jun 2019, at 13:39, Mutz, Marc via Development
<development@qt-project.org> wrote:
[...]
You are equating Qt users and Qt implementers. You can maintain the Qt
API, but use more efficient data structures in the implementation. You
seem to be implying that these two things cannot be separated.
The comment above was aimed at our APIs and towards our users, not
about how we implement things internally.
Ok, clash of context. This subthread was about the exact change in
QtWayland, though, and that's internal implementation.
None of the changes where I replaced QMap changes the public API at
all (except adding an overload because we can). No user is affected by
this in any way, except that they have a few pages of memory more free
than before.
Please explain to me how any of those changes makes _users_ of Qt have
to revert to the STL?
And please explain to me how it can possibly be worthwhile to generate
8KiB of code _just_ to not have to use lower_bound? Which argument
could *possibly* be made against a lower_bound? Esp. seeing as many
attempts to write one by hand have failed. I remember a bug about
shortcuts being mapped to the wrong key, because the hand-rolled
binary search was unstable.
You have in general been advocating lately to remove/deprecate lots of
Qt API in favour of STL. This is what I referred to, not
implementation details in our code.
Ok, noted.
Removing the usage of a QMap for a mapping of 5 values is the right
thing to do. Whether we need lower_bound() for that and a sorted list,
or simply an unsorted vector where we iterate until we find the right
value doesn’t matter too much in this case. Performance wise both
would probably be equivalent :)
I'm sorry, but we have a lot of code that is less readable than any of
the changes I uploaded. It just cannot be an argument to say that it's
unreadable because it uses an STL algorithm. This sentiment has caused
so very, very many quadratic loops because people get the impression
that std::remove_if is toxic, and in each one the solution was to use
remove_if, because the hand-rolled alternative would be totally
unreadable.
Nobody here said that. I also believe those changes that you proposed
got approved to the largest degree.
This is not the impression I get from
https://codereview.qt-project.org/c/qt/qtwayland/+/264069, or
https://codereview.qt-project.org/c/qt/qtbase/+/264129 vs.
https://codereview.qt-project.org/c/qt/qtbase/+/264161
If you read what I said earlier, I also said that in most cases better
data structures and algorithms will lead to more readable code. I am
not opposed to using those in Qt, quite the opposite.
Good.
I'm sad to see that Qt devs think so lowly of themselves as to be
unable to understand a piece of code that uses an STL algorithm.
Really. I'm out of words.
Nobody ever said that as far as I can tell. What people said is that
there are cases, where a loop is just as efficient and easier to read
(e.g. when searching for an entry). And in that case it’s fine to
simply use that. So please stop turning the meaning of what people
said around like that.
I was a poignant remark, yes, but that's _exactly_ the feedback I get
from the reviews I quoted above: Dropping QMap is acceptable when the
API is unchanged (as with the array replacing the map), but totally not
acceptable (b/c/o unreadable) if it contains lower_bound or find_if. To
the point that the 'better' choice is to replace QMap with QHash -
because they have the same API. I'd ROTFL if I wasn't crying.
Thanks,
Marc
_______________________________________________
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
