<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 1, 2016 at 8:36 PM, Thiago Macieira <span dir="ltr"><<a href="mailto:thiago.macieira@intel.com" target="_blank">thiago.macieira@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Premises not under discussion:<br>
<br>
        Qt source code is product and meant to be read by our users<br>
        Qt source code must be clean and readable<br>
<br>
The above is not up for debate.<br>
<br>
For some time now, we've had a flurry of changes to Qt source code that uses<br>
the Standard Library's containers and algorithms, in the name of performance<br>
and often code size gains.<br>
<br>
I'm not disputing that there is a gain. But I am wondering about the trade-off<br>
we're making with regards to readability. For example, I was just reviewing<br>
this code:<br>
<br>
    if (d->currentReadChannel >= d->readHeaders.size()<br>
        || d->readHeaders[d->currentReadChannel].empty()) {<br>
        Q_ASSERT(d->buffer.isEmpty());<br>
<br>
The use of the Standard Library member "empty" is highly confusing at first<br>
sight because it does not follow the Qt naming guidelines. It's even more<br>
confusing because the next line has "isEmpty". When I read this code, I had to<br>
wonder if that "empty" was a verb in the imperative, meaning the author was<br>
trying to remove all elements from the container.<br>
<br>
I had to look up the definition of readHeaders in the review and note that it<br>
was a std::deque, not a Qt container.<br>
<br>
What do we do?<br>
<br>
Option 1:<br>
Not use Standard Library containers, just use the Qt containers as they exist.<br>
<br>
Option 2:<br>
Create new Qt containers to have the same complexity as Standard Library<br>
containers, but following the Qt naming conventions. Possibly with implicit<br>
sharing.<br>
<br>
Option 3:<br>
Create Qt API wrappers for those containers like std::deque, adding only a few<br>
inline functions to match the Qt-style API where the Standard Library API<br>
deviates. Examples are:<br>
        empty           ->      isEmpty<br>
        push_back       ->      append<br>
        front           ->      first<br>
        pop_front       ->      takeFirst<br>
        cbegin          ->      constBegin<br>
        cfind           ->      constFind<br><br></blockquote><div>I don't get why you would be confused by - for instance - the empty method.</div><div>STL has that method, but so does Qt [1] apparently in an attempt to be compatible with the STL API.</div><div><br></div><div>Even if you don't want to learn or know how the STL API looks like, it should not matter since the very same members also apply on Qt containers with the same .</div><div>This is the case for all your examples in Option 3.</div><div><br></div><div>Imho it would be nice - when using Qt and STL containers in the same statement - to use common functions that mean the same in both API's. Thus in your example both should have been empty() calls, even on the Qt container to be consistent. But that's just my opinion :)</div><div><br></div><div>[1] <a href="http://doc.qt.io/qt-5/qlist.html#empty">http://doc.qt.io/qt-5/qlist.html#empty</a></div></div></div></div>