[Development] Views

Eike Ziller Eike.Ziller at qt.io
Tue May 21 11:25:24 CEST 2019



> On 21. May 2019, at 08:32, Mutz, Marc via Development <development at qt-project.org> wrote:
> 
> On 2019-05-20 23:43, André Pönitz wrote:
>> On Mon, May 20, 2019 at 11:23:13PM +0200, Mutz, Marc via Development wrote:
>>> On 2019-05-20 23:21, André Pönitz wrote:
>>> > > > Exhibit A:
>>> > > >
>>> > > >      foo().contains(x)
>>> > > >
>>> > > >
>>> > > > Exhibit B:
>>> > > >
>>> > > >      {
>>> > > >          ... container = foo();
>>> > > >          std::find(container.begin(), container.end(), x) !=
>>> > > > container.end();
>>> > > >      }
>>> > >
>>> > > And now do the same thing [...]
>>> >
>>> > No, I won't.
>>> >
>>> > You were claiming something universally valid (\forall x "There is
>>> > no difference ...")
>>> I never said that there's no difference. I said there's no difference in
>>> readability. Don't confuse familiarity with simplicity (or readability).
>> Indeed, you claimed only "no difference in readability".
>> But I wouldn't agree that there is "no difference in readabilty" between
>> Exhibit A and B above.
>> In case B one e.g. would need to check whether all _four_ occurances of
>> 'container' are actually the same. An issue that does not exist in case A.
> 
> By that line of reasoning, the change from
> 
>   Q3Slider *sl = new Q3Slider(0, 100, 50, 10, 1, this);
> 
> to
> 
>   Q4Slider *sl = new Q4Slider(this);
>   sl->setRange(0, 100);
>   sl->setValue(50);
>   sl->setPatheStep(10);
>   sl->setStep(1);
> 
> was also wrong. I now need to check that the calls on the slider are always on the same object.

Readability is a thing of many aspects, so you cannot take a single aspect and use that to compare completely different pieces of code that do completely different things.

You’ll have to stay at the specific example and explain why there is “no difference in readability” in _that_ example.

> You are confusing familiarity with simplicity.

You are mixing together readability and (some definition of) simplicity.

> It's simple that in the STL I can search for a value or a predicate condition at the change of just an '_if'. Container::contains(), OTOH, is just familiar (from previous Qt versions or other languages). It's neither simple in the sense that it would be universal (as find/find_if is) nor is it efficient: QVector<QString>::contains() is not overloaded on QLatin1String, e.g. Std::find(), OTOH, will transparently use the QLatin1String, avoiding the creation of a temporary QString. If contains() turns out to be a performance bottleneck in profiling,

> the user can't do anything about it and needs to go to the universal mechanism (std::find).

Good. Use the API that is most readable for the use case.

> This does not make the API simple to use. Maybe for people coming from other languages. It introduces two different ways to do essentially the same thing, advocating the ways that only works in a particular corner-case and refusing to show the universal way (https://doc.qt.io/qt-5/qvector.html#indexOf). But eventually, these developers _will_ need to search by a predicate, too, and then they will fall back to a raw loop, sometime, in the case of removing, introducing quadratic complexity. Had the Qt API and the docs not lied to them by permanently showing contains() without mentioning that it's but a short-cut for a corner-case, but shown them std::find or std::remove instead, the developer would have been empowered.

That is a nice suggestion on how to improve the documentation. Which doesn’t have much to do with readability or simplicity of the API.

> As-is, Container::contains() dumbs the novice down by telling him a fairy-tale that isn't applicable to actual professional work.

And yet std maps and sets will have Container::contains in C++20.

-- 
Eike Ziller
Principal Software Engineer

The Qt Company GmbH
Rudower Chaussee 13
D-12489 Berlin
eike.ziller at qt.io
http://qt.io
Geschäftsführer: Mika Pälsi,
Juha Varelius, Mika Harjuaho
Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B



More information about the Development mailing list