Opened 8 years ago

Closed 8 years ago

#6204 defect closed fixed (fixed)

Complex, multiline, multi-lambda expressions in ampclient.py example make it more difficult than necessary to follow

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: Branch: branches/simpler-ampclient-example-6204
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

We all know it is possible to write code like:

    d1 = ClientCreator(reactor, amp.AMP).connectTCP(
        '127.0.0.1', 1234).addCallback(
            lambda p: p.callRemote(Sum, a=13, b=81)).addCallback(
                lambda result: result['total'])

And for many people, it's even possible to read such code. However, many more people have an easier time reading something where each step is separate and where lambda is not used to define a function inside a call to addCallback. Particularly since the reader of any of these examples is already trying to learn how to use AMP, they don't need to be given the extra burden of untangling this less-than-perfectly readable style.

Change History (5)

comment:1 Changed 8 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/simpler-ampclient-example-6204

(In [36525]) Branching to 'simpler-ampclient-example-6204'

comment:2 Changed 8 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

I think that's better. There are still problems with the example, such as having no explanation about how to use it or what behavior to expect from it, or the continuing use of ClientCreator instead of endpoints, or manual reactor management. I think those would all make good tickets separate from this one, which makes one well-defined improvement to the readability of the example.

comment:3 Changed 8 years ago by teratorn

Owner: set to teratorn

comment:4 Changed 8 years ago by teratorn

Keywords: review removed
Owner: changed from teratorn to Jean-Paul Calderone

I've reviewed the proposed change to ampclient.py and I like the new style. It's worth noting the "inline" placement of adding Deferred callbacks. Haven't seen that much - and it does seem to help with readability.

I guess we don't have any test case policy for example code :/ (it does appear to work, still)

OK for .misc entry, but if any other improvements take place we should probably mention the improved AMP examples in a news file.

Please merge.

comment:5 Changed 8 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [36534]) Merge simpler-ampclient-example-6204

Author: exarkun Reviewer: teratorn Fixes: #6204

Simplify the Python style used in the AMP client example.

Note: See TracTickets for help on using tickets.