python-twitter
python-twitter copied to clipboard
1000 lines of very verbose code can be easily cleaned up
Most of the objects use getters and setters in a completely unnecessary way, adding literally 1000 lines of noise to the code.
(For an explanation of why they are unnecessary, see http://dirtsimple.org/2004/12/python-is-not-java.html )
Many other methods on the simple data objects have lots of duplication and could be simplified and shortened a lot by using some of Python's dynamic features, like introspection etc. (Especially AsDict
, __eq__
etc.)
Would you merge these kind of changes if I submitted a pull request? Or after the move to API v1.1?
I worked on the same project as PJE that caused him to write that article (and I remember talking to him about it) so yes I'm aware of the pitfalls of this method.
The original author of python-twitter used a code generator to create all of this based on the protobuf description that he created for the twitter api - so that's why you see lines and lines of code that seem to scream "WTF".
All that aside, it has always been in the back of my mind to see if most of it could be simplified. I am leery of real heavy use of introspection and "fancy" methods - only from a code maintenance point of view.
So yes, if you want to dig in and start with some of the secondary classes so we can start haggling over some of the details, I would definitely welcome it. I would use it as the starting point for the v1.1 changes since that is a perfect time to break things.
thanks!
I've created a branch named api_v1.1 for us to start working with
+1 Thanks a lot, it's gonna be definitely useful!!
I'm also cautious of lots of introspection and fancy tricks, but in a few cases it can actually clarify the code a lot - for example, the AsDict
methods could almost be replaced with self.__dict__.copy()
, and the attributes which need special handling are then more obvious.
Also, repeated long list of attributes can obscure intention and hide bugs - for example, the attribute that appear in Status.__eq__
are not all the attributes, but it's not obvious if this is a bug or deliberate or why.
Obviously I would split my changes up so that you could pick the ones you want.
Don't worry about splitting it up too much, heck, if your interested in helping and this is what you want... then I would be foolish to start throwing roadblocks at ya ;)
I would just like to start with some smaller examples so I can get a feel for what your talking about and that the others who are also contributing can review. Once we get comfortable with each other, then the fun starts :)
Regarding properties, do we support python < 2.6? The @x.setter, @x.getter stuff is only in >= 2.6 and if the code is changed to properties it would make sense to use these from the beginning.
that's a good question, I personally don't mind with setting the bar to 2.6 once we get the core v1.1 work done.
that lets use have that version for the folks who don't (or can't) move to 2.6+
I tried something at sebastianw/python-twitter@c7892489b38867163c69ba16edae38e3394fd97a. Replaced the Status class with a much shorter version. It seems to work but I'm not sure if that is the way to go forward with the other classes. Coments welcome.