--===============2018432078181649056==
Content-Type: multipart/signed; boundary="nextPart27288286.yrKkC6tzL9"; micalg="pgp-sha1"; protocol="application/pgp-signature"


--nextPart27288286.yrKkC6tzL9
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"

On Thursday, December 18, 2014 12:03:01 PM Christian Mollekopf wrote:
> Hey Daniel,
>=20
> I meanwhile took a look at the async code, and here's the feedback I =
have so
> far.

Awesome, thanks!

>=20
> First off though, nice job! I think it goes very much in the right di=
rection
> =3D)
>=20
> So:
> * I think it would be nice to remove waitForFinished from FutureGener=
ic,
> offering only a callback. It keeps the baseclass simple and clean, an=
d it's
> easy to implement a little wrapper that provides blocking waiting if
> necessary.

We have FutureWatcher for the callback, and having a "BlockingFutureWat=
cher"=20
does not make much sense IMO.

I don't think it really matters whether the baseclass is simple or not:=
 the=20
only reason we have FutureBase is that I need to store a non-templated =
pointer=20
to Future in ExecutorBase (which is non-templated itself), otherwise it=
 would=20
all be in Future.

> * Currently you're copying all result values to pass it on to the nex=
t step,
> which is something I hoped to avoid. I think setFinished should take =
the
> value and directly call the continuation (should be possible from wha=
t I've
> seen).

We can optimize this using the move semantics etc. I was just being laz=
y :-)

>=20
> * The final value should be consumable by a continuation:
>=20
> job.exec().then<QString>(
> =09//final continuation consuming the value
> )

+1, I like this. Not sure how to do it yet, but I'll figure out somethi=
ng :)

>=20
> The continuation would in this case also replace the FutureBase::isFi=
nished
> acessor I think.

Probably, but there's no harm in keeping it around. We need the state=20=

internally anyway.

>=20
> If you wanted a future that stores the value you still could:
> MemoryFuture<QString> finalFuture(future2);
>=20
> The memory future would internally copy the value from the continuati=
on,
> perhaps emit a signal when done, and provide isFinished().
>=20
> * We may want to add support for more convenient composition:
>=20
> getJob1().then(getJob2())
>=20
> Currently you have to do something like:
>=20
> getJob1().then([this](const QString &arg, Async::Future<void> &future=
) {
>   getJob2().exec().then<void>([future]() {
> =09=09future.setDone();
> =09}
> });
>=20
> Which is fine for now and works nicely with kjobs, but perhaps it wou=
ld be
> worth it to improve on that sometime? Of course the convenient compos=
ition
> as above would only work if the result type of the first job and the
> argument type of the second job are a perfect match, so perhaps it's =
also
> not worth it.

Indeed. I used to have overloads for then(), each() etc. that accepted =
Job as=20
an argument, but they got lost over the time. I'll see if they can be b=
rought=20
back.

>=20
> * Error handling is so far missing. Each step should be able to provi=
de an
> error handler, and by default errors should bubble up, so you only ha=
ve to
> handle errors at the end (probably as an error handler continuation o=
f the
> future).

Yep, I hoped to do it this week, but so far spent most time visiting re=
latives=20
:D Will work on that.

>=20
> * We probably should have a way to abort a job (if the job supports t=
hat of
> course).

ditto

>=20
> * Execution is so far tied to the event-loop, and I'm wondering wheth=
er we
> should also support non-eventloop execution, i.e. to be executed in a=

> thread. Can we specialized the executors for that?

Indeed that should be rather easy. We still need the eventloop though, =
you=20
probably don't want a blocking wait for Future to finish.

>=20
> Otherwise it looks really nice =3D) Let me know what you think about =
the
> points above and whether you plan on working on them, otherwise I mig=
ht
> give it a try as well ;-)

It's a small codebase and since it's all template magic, even small cha=
nges=20
tend to break everything :D I will first do the error handling and abor=
ting,=20
then we can move on with the remaining stuff.

Dan

>=20
> Cheers,
> Christian

=2D-=20
Daniel Vr=E1til | dvratil@redhat.com | dvratil on #kde-devel, #kontact,=
 #akonadi
Software Engineer - KDE Desktop Team, Red Hat Inc.

GPG Key: 0xC59D614F6F4AE348
Fingerprint: 4EC1 86E3 C54E 0B39 5FDD B5FB C59D 614F 6F4A E348
--nextPart27288286.yrKkC6tzL9
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAABAgAGBQJUkuBwAAoJEMWdYU9vSuNIjFAH/i9UgDJ6yVisoNp52AgrEl1J
WIk04pdrozEkDujyroCkmALoEpLYLpLjlf9gRvS7JV++t0rLGpGowiVWU2c6wNRx
5b0S3C0cbDHubt+NXQ8beRauVbfncEOf4MFuUuBfNV43Bu/Go8ozZ7Uz4qzUYdhq
kYoR61stKzOg7Y0SZH0yECjAV/yTu2IhKj07R0lJ0V6r6Gg4igQFWzg+LTUjwvb3
9oMaqvJj5lDf/f20Qm0HsK1EpqAZZ9sll5xAjhz32mBcasPaqrx6p+cbBNRIE3OZ
JLCTslBUVDxeDev1WjgsL80WlAhiSZ8J46NtxdBCFPB4FLSOkGxPOnYZcuzExTk=
=b0nT
-----END PGP SIGNATURE-----

--nextPart27288286.yrKkC6tzL9--


--===============2018432078181649056==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
--===============2018432078181649056==--