Opened 10 years ago

Last modified 10 years ago

#3840 enhancement new

twisted.words.protocols.irc contains a lot of duplication of constant values

Reported by: Jonathan Jacobs Owned by: Jonathan Jacobs
Priority: normal Milestone:
Component: words Keywords: irc
Cc: Branch:
Author:

Description

All constants appear twice, once in the module as literal variable assignments and again in the symbolic_to_numeric dictionary.

Attachments (2)

remove-irc-constant-dups.patch (7.7 KB) - added by Jonathan Jacobs 10 years ago.
remove-irc-constant-dups-1.patch (7.5 KB) - added by Jonathan Jacobs 10 years ago.

Download all attachments as: .zip

Change History (5)

Changed 10 years ago by Jonathan Jacobs

comment:1 Changed 10 years ago by Jonathan Jacobs

Keywords: review added
Owner: Jonathan Jacobs deleted

Generate symbolic_to_numeric dictionary by iterating globals(). Also sorted constants according to their value.

Changed 10 years ago by Jonathan Jacobs

comment:2 Changed 10 years ago by Jonathan Jacobs

Actually attach valid diff.

comment:3 Changed 10 years ago by Glyph

Keywords: review removed
Owner: set to Jonathan Jacobs

Thanks for the patch! It would be nice to clean this up. I'm sure that the lists will get stale and desynchronized as people add and remove stuff, if they haven't already.

Unfortunately, my experience with locals() in python has taught me to be highly suspicious of functions that inspect the stack. I am concerned that either in some obscure situation, on a different runtime, or in some future version of Python, this trick will not work as expected. I can't think of a reasonable way to write a unit test to ensure that this does not happen, as everything I can think of involves duplicating the entire list into the test.

An ideal solution here would be to deprecate all the global values to declutter that namespace, and have a dedicated object for representing the list of enumerated IRC status values, which supports iteration and classification and potentially other stuff. The tricky part is deprecating all of the synonyms, which we need to keep around for a while for compatibility's sake.

If you have any better ideas I'd love to hear them :).

Note: See TracTickets for help on using tickets.