djmitche/500lines — pull request #2 by Ashald (2014-03-14 09:41:19+00:00)
Title: Code review v1

OK, so as promised I'm sending a PR with refactoring that I've done while I was reading 'Paxos Made Moderately Complex' and learning the Paxos implementation. This refactoring can be considered as a code review.

The main changes is:
* PEP8 fixes (though not all issues fixed)
* code style consistency (same things should be done with same code)
* code simplifications (removed some redundant constructions)
* comments and TODO
* distinguishing between public and non-public class members (all class members that weren't used from outside of a class I've prefixed with the underscore character that's so it is easier to understand which part of code is a 'public API' and which part of code is used only internally by other class methods)

As you can see, there is a lot of commits as I tried to make a lot of small changes so it will be easier to grasp the essence of the changes.

Additionally, there are few issues that should be considered though I haven't left any comments in code regarding them:
* broken code should be removed to reduce overall amount of code - I understand that this code will not be used, but I have spent some time to understand the purpose of this code and why it is broken;
* there is a common pattern to delay a function execution using timer; in such cases there are parts of code that introduce new class fields only for purpose of passing some data into delayed function - it would be great to consider using functools.partial instead as it will reduce amount of class fields and code will become easier to understand.
* such things as _Ballot(-1, 0, unique_id)_ and _Ballot(-1, -1, -1)_ seems to be a magic - it is not clear what meaning have ballots with such fields - we should use something like a factory method with sensible name to construct such objects;
* there are a lot of string literals with Paxos commands - 'PROPOSE', 'PROMISE', 'ACCEPT' etc - they should be moved to constants;