[Twisted-Python] twisted mail client code review
![](https://secure.gravatar.com/avatar/4bd4e03f4f5e45e03c09b08d3ad01e78.jpg?s=120&d=mm&r=g)
Hello guys, this are my first steps into twister and this is my first app with twister, using examples from the twisted documentation and reading it a lot ( interfaces are still not for me ), I've come to this code: http://pastebin.com/m2d6c35df My goal for now is to create my own client and be able to retrieve mail :) well it's done, but my question to the experts is to know your opinions about my code, to see if I can implement any best practice in order to have a decent app, in fact to hear from you how do you find the code PD: Sorry for my English, saludos desde Costa Rica. Thanks a lot ! -- http://celord.blogspot.com/
![](https://secure.gravatar.com/avatar/152986af8e990c9c8b61115f298b9cb2.jpg?s=120&d=mm&r=g)
On Tue, 29 Dec 2009 14:55:36 -0600 César García <celord@gmail.com> wrote:
In general, it looks pretty good! I do have some comments, though: - Occasionally when you use a docstring, the indenting is inconsistent, like this: """ docstring goes here """ Obviously it still works, but you'll be asked to fix it if you ever contribute code to pretty much any open-source project, so you might as well get in the habit now. - You import twisted.internet.reactor in main() and in ebConnection(); you might as well just import it at the top of the file along with all the other imports. - The reason people put code in a main() function rather than just putting it at the end after 'if __name__ == "__main__":' is so that they can write tests for it, or otherwise call it from other code. However, because you call reactor.run() from your main() function, it can't be called from other code - you might want to move the reactor.run() call to the bottom of the file. - In ebConnection() you set up logging to stdout, then log the error, then stop the reactor - but there are various other places where you log things with print statements. You might as well set up logging at the bottom of the file as well, then use "log.msg()" where you used to use "print". All pretty minor things, really - well done!
![](https://secure.gravatar.com/avatar/4bd4e03f4f5e45e03c09b08d3ad01e78.jpg?s=120&d=mm&r=g)
Timothy, thanks a lot, it's good to know that I am on the right way, I'll follow your advices !! And now reading again the documentation about TCPclients, I'll try to modify the code and move the callbacks inside the Protocol class and move all the persisten config to the factory Thanks ! 2009/12/30 Timothy Allen <screwtape@froup.com>:
![](https://secure.gravatar.com/avatar/152986af8e990c9c8b61115f298b9cb2.jpg?s=120&d=mm&r=g)
On Tue, 29 Dec 2009 14:55:36 -0600 César García <celord@gmail.com> wrote:
In general, it looks pretty good! I do have some comments, though: - Occasionally when you use a docstring, the indenting is inconsistent, like this: """ docstring goes here """ Obviously it still works, but you'll be asked to fix it if you ever contribute code to pretty much any open-source project, so you might as well get in the habit now. - You import twisted.internet.reactor in main() and in ebConnection(); you might as well just import it at the top of the file along with all the other imports. - The reason people put code in a main() function rather than just putting it at the end after 'if __name__ == "__main__":' is so that they can write tests for it, or otherwise call it from other code. However, because you call reactor.run() from your main() function, it can't be called from other code - you might want to move the reactor.run() call to the bottom of the file. - In ebConnection() you set up logging to stdout, then log the error, then stop the reactor - but there are various other places where you log things with print statements. You might as well set up logging at the bottom of the file as well, then use "log.msg()" where you used to use "print". All pretty minor things, really - well done!
![](https://secure.gravatar.com/avatar/4bd4e03f4f5e45e03c09b08d3ad01e78.jpg?s=120&d=mm&r=g)
Timothy, thanks a lot, it's good to know that I am on the right way, I'll follow your advices !! And now reading again the documentation about TCPclients, I'll try to modify the code and move the callbacks inside the Protocol class and move all the persisten config to the factory Thanks ! 2009/12/30 Timothy Allen <screwtape@froup.com>:
participants (2)
-
César García
-
Timothy Allen