I took some time to look at current unity from svn and here are some comments.
general comments:
- Timer class should live in its own module
client/Battle.py:
- gtk columns, renderers, etc. should probably live in lists, as having one variable for each increases clutter, but more importantly, you then don't have to make ten calls to some_method, just one in a loop.
- popup_menu: don't know the internals of gtk, but does it really have to be created on the fly _every_ time?
client/Lobby.py:
- a HUGE elif chain. this is seriously ineffcient, consider using a lookup table like this:
Code: Select all
lookupTable ={'PONG': self.client.messageQueue.put, ...}
...
lookupTable[command[0]](command)
This will perform much better and will be easier to maintain. Alternatively, you could make a set and check for existence:
Code: Select all
commandSet = set(('PONG', ...))
...
if command[0] in commandSet:
doStuff
The second way should perform the same as the first, which to choose is a matter of taste.
- isSomething: could be reduced to one-liners without loss of clarity:
Code: Select all
status = self.status
if (status & (1 << 6)) != 0:
return True
else:
return False
tranforms into
Code: Select all
F_BOT = 1<<6
...
return bool(self.status & F_BOT)
- multiple something != None. This isn't that wrong, but isn't that correct either. The pythonic way is
- not very pretty:
Code: Select all
cmdToSend[:19] == 'UPDATEBATTLEDETAILS'
it is a little bit too much work for the programmer to count how long the command is :) that's what startswith is for:
Code: Select all
cmdToSend.startswith('UPDATEBATTLEDETAILS')
That's it for now, hope it's of some use.
