Changes between Version 1 and Version 2 of CodingStandards


Ignore:
Timestamp:
2009-05-17T00:29:39Z (15 years ago)
Author:
zooko
Comment:

edit it down, remove some bits that are not likely to come up for most programmers

Legend:

Unmodified
Added
Removed
Modified

  • CodingStandards

    v1 v2  
    66=== compatibility ===
    77
    8 Tahoe requires Python v2.4 or greater. No effort should be made to offer compatibility with versions of Python older than 2.4. Effort should be made to work with the most recent release of Python v2.x, and with every release between v2.4 and the most recent release.
     8Tahoe requires Python v2.4.2 or greater. No effort should be made to offer compatibility with versions of Python older than 2.4.2. Effort should be made to work with the most recent release of Python v2.x, and with every release between v2.4.2 and the most recent 2.x.
    99
    1010=== naming and layout ===
    … …  
    3838=== truths and falsehoods ===
    3939
    40  * Always use {{{True}}} or {{{False}}} if that is what you mean -- never write a literal {{{0}}}, {{{1}}}, or {{{None}}} to indicate a boolean value.
    41  * Never use the literals {{{True}}} or {{{False}}} in conditional expressions -- instead just write the expression which will evaluate to true or false. For example, write {{{if expr:}}} instead of {{{if expr == True:}}} and {{{if not expr:}}} instead of {{{if expr == False:}}}.
     40 * Don't use the literals {{{True}}} or {{{False}}} in conditional expressions -- instead just write the expression which will evaluate to true or false. For example, write {{{if expr:}}} instead of {{{if expr == True:}}} and {{{if not expr:}}} instead of {{{if expr == False:}}}.
    4241 * Use the fact that empty sequences, empty strings, empty dicts, {{{0}}}, and {{{None}}} all evaluate to false. Write {{{if not items:}}} instead of {{{if len(items) == 0:}}}.
    4342 * But if your intent is to test for {{{None}}} instead of to test for "any false thing", then write it out as {{{if thing is None:}}}.
    … …  
    4544== advanced idioms ==
    4645
    47 === assertion policy ===
    48 
    49 One axis of interest is how time-consuming the checks are. Many precondition
    50 checks can cause typical runtime to explode to O(n^2^) or O(n^3^), for example
    51 {{{SortedList.__contains__}}} called {{{_assert_invariants}}} which took
    52 O(n log n) each time, when {{{__contains__}}} ought to be O(log n). A caller who
    53 was expecting {{{if b in list}}} to take O(log n) could easily wind up turning
    54 their O(n log n) routine into O(n^2^) or worse.
    55 
    56 Another axis is "who could cause it to fail": some checks are looking only at
    57 internal state. For example, if {{{SortedList._assert_invariants}}} fails, it
    58 indicates a problem in some {{{SortedList}}} method. Other checks are
    59 enforcing the external API, like those which do typechecks on input
    60 arguments. Even after the {{{SortedList}}} developer has gained confidence in
    61 the code and decides that internal checks are no longer necessary, it may be
    62 useful to retain the external checks to isolate usage problems that exist in
    63 callers.
    64 
    65 We decided upon a couple of policies for the future:
    66 
    67  * The general rule is that nodes must be functional for light traffic even
    68    when the assertions are turned on. When assertions are turned off (-O),
    69    nodes must be functional for heavy traffic.
    70 
    71  * Time-consuming internal checks: once the code is working properly,
    72    consider removing them, but they may be left in place as long as they
    73    use {{{assert}}} (the form which gets turned off when -O is used).
    74 
    75  * Cheap internal checks: once the code is working properly, consider
    76    removing them, but it is less of a concern than the time-consuming ones.
    77    If they really are cheap, use {{{_assert}}} (the unconditional form
    78    that gets used even with -O).
    79 
    80  * Time-consuming external checks: maybe leave them in place, but always
    81    use {{{assert}}} so they will not be used with -O.
    82 
    83  * Cheap external checks: leave them in place, using the unconditional
    84    {{{_assert}}}
    85 
    86  * Production grids could run with -O (in practice, the allmydata.com production grid runs without -O, because there are no expensive checks in the current codebase).
    87 
    88  * Testing grids might run without -O in order to detect more bugs.
    89 
    90  * Local developer tests will probably not use -O, and developers should be
    91    prepared to experience the same CPU load problems if they subject their
    92    nodes to real traffic levels. Developers can use -O to turn off everyone
    93    else's checks, use {{{_assert}}} on their own code to enable their own
    94    assertions, and then subject their nodes to heavy traffic, as long as they
    95    are sure to change their checks to use {{{assert}}} (or remove them
    96    altogether) before committing.
    9746
    9847
    … …  
    14392Now you can put {{{assert self._assert_consistency()}}} everywhere in your class where the class ought to be in an internally consistent state. For example, at the beginning of every externally-callable method. This technique can be very valuable in developing a complex class -- it catches bugs early, it isolates bugs into specific code paths, and it clarifies the internal structure of the class so that other developers can hack on it without subtle misunderstandings.
    14493
     94==== assertion policy ====
     95
     96One axis of interest is how time-consuming the checks are. Many precondition
     97checks can cause typical runtime to explode to O(n^2^) or O(n^3^), for example
     98{{{SortedList.__contains__}}} called {{{_assert_invariants}}} which took
     99O(n log n) each time, when {{{__contains__}}} ought to be O(log n). A caller who
     100was expecting {{{if b in list}}} to take O(log n) could easily wind up turning
     101their O(n log n) routine into O(n^2^) or worse.
     102
     103Another axis is "who could cause it to fail": some checks are looking only at
     104internal state. For example, if {{{SortedList._assert_invariants}}} fails, it
     105indicates a problem in some {{{SortedList}}} method. Other checks are
     106enforcing the external API, like those which do typechecks on input
     107arguments. Even after the {{{SortedList}}} developer has gained confidence in
     108the code and decides that internal checks are no longer necessary, it may be
     109useful to retain the external checks to isolate usage problems that exist in
     110callers.
     111
     112 * The general rule is that nodes must be functional for light traffic even
     113   when the assertions are turned on. When assertions are turned off (-O),
     114   nodes must be functional for heavy traffic.
     115
     116 * Time-consuming internal checks: once the code is working properly,
     117   consider removing them, but they may be left in place as long as they
     118   use {{{assert}}} (the form which gets turned off when -O is used).
     119
     120 * Cheap internal checks: once the code is working properly, consider
     121   removing them, but it is less of a concern than the time-consuming ones.
     122   If they really are cheap, use {{{_assert}}} (the unconditional form
     123   that gets used even with -O).
     124
     125 * Time-consuming external checks: maybe leave them in place, but always
     126   use {{{assert}}} so they will not be used with -O.
     127
     128 * Cheap external checks: leave them in place, using the unconditional
     129   {{{_assert}}}
     130
     131 * Production grids could run with -O (in practice, the allmydata.com production grid runs without -O, because there are no expensive checks in the current codebase).
     132
     133 * Testing grids might run without -O in order to detect more bugs.
     134
     135 * Local developer tests will probably not use -O, and developers should be
     136   prepared to experience the same CPU load problems if they subject their
     137   nodes to real traffic levels. Developers can use -O to turn off everyone
     138   else's checks, use {{{_assert}}} on their own code to enable their own
     139   assertions, and then subject their nodes to heavy traffic, as long as they
     140   are sure to change their checks to use {{{assert}}} (or remove them
     141   altogether) before committing.
     142
     143
    145144=== configuration ===
    146145
    … …  
    149148 * Do not implement configuration files for modules or libraries -- code that is going to be used by other code. Only applications -- code that is going to be used by humans -- have configuration files. Modules and libraries get "configured" by the code that calls them, for example by passing arguments to their constructors.
    150149 * If there are constant values which end-users do not need to modify, then do not make them configurable, but put them in all-caps variables at the beginning of the Python file in which they are used.
    151  * Design algorithms so that they have as few "voodoo constants" and "tweakable parameters" as possible. If you find yourself needing to add more and more special cases to handle failures of the basic algorithm, this might indicate that the basic algorithm ought to be replaced with one that doesn't have so many edge cases.
     150 * Design algorithms so that they have as few "voodoo constants" and "tweakable parameters" as possible.
    152151
    153152==== how to implement configuration ====
    … …  
    165164        self.maintainertype = confdict.get("MAINTAINER", "rnd").lower()
    166165        self.backendtype = confdict.get("BACKEND", "flat").lower()
    167 
    168 blockstore = BlockStore(confdict)
    169166}}}
    170167
    … …  
    177174        self.maintainertype = maintainertype
    178175        self.backendtype = backendtype
    179 
    180 maxspace = confdict.get('MAX_MEGABYTES')
    181 if maxspace is not None:
    182     maxspace = int(maxspace) * 2**20
    183 blockstore = BlockStore(maxspace, confdict.get('PATH', ""), confdict.get('MAINTAINER', 'rnd').lower(), confdict.get('BACKEND', 'flat').lower())
    184176}}}
    185177.
    186178
    187 == Twisted gotchas ==
    188 
    189 The standard-library os.popen() function is not compatible with the way Twisted uses SIGCHLD. The usual symptom is that a read() or readlines() on the filehandle returned by
    190 os.popen() fails with EINTR "Interrupted System Call". I'm not sure exactly why this happens, and worse yet it only happens intermittently (-warner 06-Dec-2005).
    191 
    192 Instead of os.popen(), you should use twisted.internet.utils.getProcessOutput(). This function returns a Deferred which fires (with the contents of stdout) when the process
    193 completes. This means, of course, that you must attach a callback to handle the output, rather than blocking the entire process while you wait for a synchronous result.
    194179
    195180== official Python standards ==