I just need to know if it's decent or not.
Well, it's not.
- You're not following PEP8, the official Python style guide
- You have spaces mixed with tabs, which can lead to nasty bugs that will really hurt you
The code is buggy and strange in many places.
def on_message(self, message): self.modDirectory = 'modules/*.py' self.files = glob.glob(self.modDirectory)
The variable is called modDirectory
, but its value is a glob. That's misleading. But the bigger problem is that modDirectory
is an instance variable, but it's not used anywhere else in the posted code. This is better:
self.files = glob.glob('modules/*.py')
There is a bug here:
data = message.split(':', 1) if len(data) == 1: cmd, args = data[0]
This assignment won't work, you'll get ValueError: too many values to unpack (expected 2)
.
What is going on here?
self.getCommand(cmd) cmd = self.getCommand(key)
First you call self.getCommand
and throw away the result? It's not clear what you're doing here and why. If the first call is necessary, that would mean that self.getCommand
has side effects, which is a bad practice in general, especially for something that looks like a getter.
What's going on here:
if cmd: if cmd:
At this point I stopped reviewing. This is just too messy and far from decent. I bet you could spot these things yourself if you give it a careful read!
Here's what you can do to improve this, very easily:
Install
pep8
(pip install pep8
). It's a command line tool that verifies specified file or an entire directory tree for PEP8 violations. As much as possible, follow it and fix everything it tells you.Install
pyflakes
(pip install pyflakes
). Another command line tool, that checks for common mistakes in Python coding practices. Use it the same way as thepep8
tool.
If you do thisuse these tools to clean up your code and post again a new question, that will have a LOT better chance of being decent.!