[Development] Views

[Mutz, Marc]

On 2019-06-06 14:04, Lars Knoll wrote:
>> On 6 Jun 2019, at 13:39, Mutz, Marc via Development 
>> <development at 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