[a somewhat rambling description of the process, techniques and some of the thinking behind a basic refactoring of a Python/Twisted program, with Python neophytes in mind]
[original tutorial] [messy source code] [refactored source code]
I’m looking at you, chatserver.py. You’re part of the tutorial I posted last week, and are rather in need of some cleanup.
What to refactor?
If you haven’t yet, take a look at chatserver.py. You’ll see that it’s quite long, somewhat messy, and that there are distinct bits of code there which are self contained (the WebsocketChat class, for example). There’s opportunity for some cleanup.
When seeing this file, I want to do three things:
- Move all of the self contained chunks of code to separate files
- Rearrange the code in this file, and any new files, to make it slightly more readable (for example, bring it closer to the PEP-8 standard)
- Keep an eye out for any other small, opportunistic, code clean-up that might make sense to include
There are three interconnected classes, with fairly separate function, and fairly readable code calling them. A first cut for a refactoring might be to move each of the classes (WebSocketChat, ChatFactory, HttpChat) into a separate module/file. So, let’s start there.
Where should Python modules go?
Technically, the answer to this question can be quite complicated. The Python module system is complex, powerful, and can handle some fairly fancy bits of organization. It might be fun to write about the details of what can be done some other time. For this post, though, I’ll only review enough to explain the refactoring I’m making.
We probably want to create some modules, to store the code we’re factoring out of the main program. We also want Python to be able to find our modules when we import them – and Python has some conventions and built in assumptions, to help make this work as easy as possible.
If you read the relevant documentation, you’ll see that, unless you go to the trouble of configuring it to do something else, Python will try and find any modules you find in the following locations:
- a disk location storing built in and system defined modules
- any modules in the directory of the currently running script
- modules installed somewhere else in the PYTHONPATH (where you might, for example, find non core libraries and other related tools)
For our case, 2. is probably the relevant choice – so we’ll go ahead and refactor the chatserver.py code into module files, stored in a subdirectory in our project.
Most often, any work and refactoring done on a Python project is restricted to the working directory of the currently running script (2. above). It’s fairly unusual to work on more than one project at a time, but common enough that most people will find themselves doing it – in which case 3. might apply – you might find yourself refactoring code into a module that’s from a project external to the one the code is a part of (say, if you’re working on a test framework for one of your projects). It’s even more unusual to worry about modifying or creating system defined modules – at best, you might run into 1. when switching between different versions of Python for compatibility and other tests.
How?
First, we’ll want a namespace for storing and referencing the classes we’re refactoring. “twisted_chat” seems a good enough namespace.
There’s a simple, 2 step, convention for defining a namespace in python – first, create a directory, and then place an file in it, called __init__.py:
bash: cd [directory where the django_twisted_chat files are checked out] bash: mkdir twisted_chat bash: touch twisted_chat/__init__.py
__init__.py can remain empty for now. It gets executed as part of the module import/initialization process, and can, if needed, do some pretty powerful bits of manipulation. At this stage in the projects’ life, it’s not required to do much, though, so I’ll ignore it for now. (We’ll almost certainly want to modify it before sending this project to a production server, though. Probably to add some code to handle module level logging and maybe some other module initialization code.)
Modules are one of the standard kinds of namespaces used by Python, and are almost always represented as directories on disk. Python source files also function as namespaces – and, since we need to place our classes somewhere, let’s create some source files. In the twisted_chat directory, create three files:
touch twisted_chat/factories.py touch twisted_chat/protocols.py touch twisted_chat/resources.py
Now, to the fun bit: move the three classes and all related, relevant code into each of the files. WebSocketChat into protocols.py, ChatFactory into factories.py, and HttpChat into resources.py.
As an aside, some programmers might disagree with this specific choice of organization. The three classes are relatively small, and separating them into three files like that seems a bit wasteful. Also, there are some fairly intimate interdependences between them, especially considering the shared lock and the shared messages dictionary. Some programmers might wait until there are a few more classes before splitting them out like this (and maybe, for now, they might store all three classes in just one file).
The most difficult part of moving the three classes out is figuring out what the “relevant code” bit is, especially when looking at longer, or more complex chunks of code. In our specific case, the extra relevant code is mostly import statements, something a Python programmer can probably do by just reading the source code.
For more complex classes, though, you might try to use an iterative approach. For example, you can move the smallest possible chunk of self contained source code (maybe just a subset of a class into a separate file), import it the new file back into the original program, and then test your program. Each time an import error shows up, correct it, until you run out of errors. This is especially easy to do if you have a solid set of tests to run against your code, once the refactoring is complete – the more complex the work, the likelier it is that you’ll introduce errors, and you won’t find them without good test coverage. Once all of your tests pass, move another small bit of code out, testing thoroughly, and repeating until the entire complex section, or class, is factored out.
As you factor out the three classes, you’ll quickly lose the reference to the shared write_lock. The class signatures will need some editing to account for this – so, change the classes as you move them, so that they take a write_lock argument when they’re initialized. Pass the lock around from one class to the other, as needed. If you’ve never refactored Python code before, it’s a worthwhile exercise to do this work now. If you’re comfortable with refactoring, read the opportunistic cleanup section at the bottom of this post, for an additional bit of refactoring you can combine into your work, saving you some time in the longterm.
As you go along, you’ll notice that the chatserver.py file will no longer depend on some of the import statements you copy over – just remove them.
Also, don’t forget to import the newly created module and source files, as necessary. Your import statements in chatserver.py will likely look like this:
from twisted_chat.factories import ChatFactory from twisted_chat.resources import HttpChat
To make sure that you’ve completed the refactoring without breaking anything, try running the chat server:
bash: python manage.py runserver & twistd -n -y chatserver.py &
then connect to it and make sure that you can still send chat messages properly:
http://127.0.0.1/chat/1 http://127.0.0.1/chat/long_poll/1
Opportunistic code cleanup
Earlier in this post, I mentioned making code more readable as a goal of this refactoring.
If you’re new to Python, you might not be aware of PEP-8 – a set of guidelines, describing a standard way to format and write Python code. If you don’t know about it, it’s worth reading. Since we’re doing all of this refactoring work, it’s worth seeing if the source can be made more readable, and also brought closer to the PEP-8 standard.
I also mentioned that I’ll keep an eye out for other refactoring work that might fit in with what we’re doing. Passing the write_lock around, you might have noticed, can be painful. So it’s probably worth while to look into refactoring our code, to find a way to not have to do that work.
A couple of hints present themselves. The write_lock is exclusively used with the shared messages dictionary, and the dictionary is already shared between all of the relevant structures. What if we were to attach the lock to the shared dictionary?
You can try doing this on your own:
- create a subclass of the standard dictionary structure
- find a way to pass the write_lock in to the structure’s initializer
- override the two methods which can be used to modify the dictionary:
class WriteProtectedDict(dict):
def __init__(self, write_lock):
def __setitem__(self, key, value):
def __delitem__(self, key):
Have you succeeded? What’s your code look like? Take a look at the django_twisted_chat git repository – this version contains all of the work from this post (the twisted_chat module and chatserver.py are the relevant bits). You can download it and read it locally if you want:
git clone https://github.com/aausch/django_twisted_chat/ demo_source cd demo_source/django_twisted_chat git reset --hard 5d1a8e5a448f86ce6da6425754f14e00bb00e9b8
P.S.
Wait, what? I put a write_lock on a system data structure… in Python!? OK. That’s… weird… I smell a bug.
P.P.S.
if you’re learning how to refactor, or are just looking for a second opinion, you might want to check out
the python questions on the codereview stackexchange site
ferretfarmer 