Refactor TwythonStreamer.on_success #223

New issue

Closed

opened 2013-06-18 21:41:32 -07:00 by extesy · 2 comments

extesy commented

2013-06-18 21:41:32 -07:00

(Migrated from github.com)

Current implementation of TwythonStreamer routes the stream in on_success handler to delete/limit/disconnect. There are also base empty implementations of these hooks.

The problem is that if you override on_success you lose that routing. Workaround to call super() is hacky because it implies exact knowledge of TwythonStreamer source code which is a bad design.

I suggest to refactor on_success routing out of that function and into the main while self.connected loop. This way you can override any handler as necessary without breaking the routing behavior and you don't need to know that one (out of 6) hooks is somehow special and does the routing.

Current implementation of TwythonStreamer routes the stream in `on_success` handler to delete/limit/disconnect. There are also base empty implementations of these hooks. The problem is that if you override `on_success` you lose that routing. Workaround to call super() is hacky because it implies exact knowledge of TwythonStreamer source code which is a bad design. I suggest to refactor `on_success` routing out of that function and into the main `while self.connected` loop. This way you can override any handler as necessary without breaking the routing behavior and you don't need to know that one (out of 6) hooks is somehow special and does the routing.

extesy commented

2013-06-18 22:44:33 -07:00

(Migrated from github.com)

I can probably create a PR for this issue if you think it's a good idea overall.

michaelhelmick commented

2013-06-19 06:34:05 -07:00

(Migrated from github.com)

Go for it! :)

Sent from my iPhone

On Jun 19, 2013, at 1:44 AM, Oleg Anashkin notifications@github.com wrote:

I can probably create a PR for this issue if you think it's a good idea overall.

—
Reply to this email directly or view it on GitHub.

Go for it! :) Sent from my iPhone > On Jun 19, 2013, at 1:44 AM, Oleg Anashkin notifications@github.com wrote: > > I can probably create a PR for this issue if you think it's a good idea overall. > > — > Reply to this email directly or view it on GitHub.

No labels

Bug

Enhancement

Feature Suggestion

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: code/twython#223

No description provided.

Rows
Columns