[Development] Qt 5.6.0 header diff: QtQuick.diff

Albert Astals Cid albert.astals at canonical.com
Fri Sep 18 10:47:55 CEST 2015


On Fri, Sep 18, 2015 at 10:10 AM, André Somers <andre at familiesomers.nl> wrote:
> Op 18-9-2015 om 09:59 schreef Albert Astals Cid:
>>
>>> New classes:
>>>
>>> * QQuickImageResponse
>>>
>>> Isn’t this class missing some way to get the status of the response? It
>>> only has an errorString() method, but no simple way to query whether it
>>> was successful or not when finished() gets emitted.
>> The docu says that "An empty string means no error.".
>>
>>
> Isn't that inconsistent with similar API's in Qt that do keep the error
> message separate from the error status?

It is indeed a bit inconsistent, but on the other hand what we usually
have is a enum that defines the error type and then errorString
function that basically translates that enum to a string. In this case
there's no enum possible so it would mean making the people that
reimplement this class have to implement both a success() and a
errorString() function.

So it would be http://paste.ubuntu.com/12447071/

Also realize this is not a class the developer consumes but a class
the developer implements, so the fewer things we give him to make a
mistake/inconsistent (like returnning an errorString while success
returns true) the better.

Where the m_success bool is basically just a !errorstring.isEmpty()

Maybe what we can do is add a non virtual bool success() const that
just returns !errorstring.isEmpty()? Not that it would add much since
the only place this would be used is in
QQuickPixmapReader::asyncResponseFinished and would mean changing

       if (!response->errorString().isEmpty()) {
           error = QQuickPixmapReply::Loading;
           errorString = response->errorString();
       } else {
           t = response->textureFactory();
       }

to

       if (!response->success()) {
           error = QQuickPixmapReply::Loading;
           errorString = response->errorString();
       } else {
           t = response->textureFactory();
       }

But anyway if you really want it changed i won't oppose.

Cheers,
  Albert

>
> André
>
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development



More information about the Development mailing list