On Tue, Mar 30, 2010 at 12:27:54PM -0000, Twisted wrote:

1. I'm pretty sure this patch, in its current form, will break in the (still supported) Python 2.4, since it uses the 2.5 generator feature where you pass in data.

Yep - I'll fix this.

2. Why not initialize the parser in the Protocol's __init__? Less code in a frequently called path.

Good idea, since there's no need anymore to check in every call of dataReceived.

3. ``stringReceived`` in docstring violates coding standard, should be a L{} or C{} or something, I forget which. So does received_string variable name.

Oops - this looks like ReST markup :-(.

4. In dataReceived, when would we expect to get StopIteration to be raised? Quick read of the generator doesn't show anywhere it actually returns, it seems to be in infinite loop.

When the generator is left by raising an exception, it will raise a StopIteration the next time someone sends data. But this is irrelevant now, since Python 2.4 does not support sending data to a generator.

5. Using long() is unnecessary, you can just use int().

Ok.

6. Hooray for more and better tests! I am uncomfortable about use of transport.disconnecting, but we've already gone down the path of using it elsewhere, and not sure what else you could do, so it's probably fine.

I might introduce a new exception (see discussion on the mailing list) - this would allow to get rid of the check for transport.disconnecting.

7. In test_receiveTwoNetstrings(self), perhaps the second string should be different than the first, e.g. "a" and "woo", to catch theoretical bugs where parser repeatedly spits out first thing it parsed.

Good idea. Thanks & best regards, Albert -- DI Albert Brandl Weiermayer Solutions GmbH | http://www.weiermayer.com 4813 Altmünster | Abteistraße 12 | Austria Fon: +43(0)720 70 30 14 | Fax: +43 (0) 7612 88081