Classes and Methods

Always use ``new-style'' Python classes (see http://www.python.org/doc/newstyle.html), as they have some subtle and important benefits over the old-style Python classes.

Always have an __init__ method for each class. Make it the first method of the class. (If there isn't anything to do, just put pass in the body.) Name, and initialize, all instance variables in the __init__ method.

Use keyword parameters in method calls, as it is more readable. In method definitions, use defaults as appropriate, but keep the design clear and understandable. If the parameter is required, just don't put a default.

Ideally, a default for a given parameter should occur in just one place, and not be repeated in multiple method or function definitions. (In the other definitions, use a required parameter instead and pass along the default from the other method or function.) For example, suppose some method has a parameter that includes a default value: replace_nulls_with=0.0. Other methods that have the replace_nulls_with parameter should require it, and use the value passed along from the method in which it was given the default. Otherwise, if you want to change the default, there would be lots of places to change. If it turns out to be awkward to follow this rule, and you really do need to give a default in lots of places, give it a name and use that name in the parameter list, e.g. replace_nulls_with=self.default_null_replacement_value.

We'll delete the following example in a bit -- but there are quite a few places in the existing code that use keyword parameters in more complex ways than need be, e.g. by setting the default to None, and then having an if in the code that tests for None and if so sets the parameter to a default. So I put in the discussion for the moment.

For example, consider the following. (Any resemblance to actual Opus code is purely coincidental.)

class _Database :
    def __init__(self, host=None, user=None, password=None, database=None, \
        environment=None, silo_path=None, show_output=True) :
        """ Returns a connection to this database.
        If database does not exist, it first creates it.  If the
        environment is given, then take host, user and pass assignments
        from it, or partially override the environment values if specific
        others are given.  'database' is actually a required parameter; if
        left as None, failure is certain, but it is named nonetheless so
        that the signature is flexible. """
        self.show_output = show_output
        if environment != None:
            self.host = environment['host_name']
            self.user = environment['user_name']
            self.password = environment['password']
        if host != None:
            self.host = host
        if user != None:
            self.user = user
        if password != None:
            self.password = password
        self.database_name = database
        .....

This has various useful defaults for the parameters, but could be improved.

class MySQLDatabase :
    """This class provides uniform access in Opus to a MySQL database ..."""
    def __init__(self, database,
            host='localhost',
            user=os.environ['MYSQLUSERNAME'],
            password=os.environ['MYSQLPASSWORD'],
            silo_path=None,
            show_output=True) :
        self.show_output = show_output
        self.database = database
        self.user = user
        self.password = password
        .....

Note that the treatment of defaults is simplified -- having optional parameters that set the values of other optional parameters should be avoided. Also, putting the defaults in the method header makes it clearer what is going on. Naming the parameter database and then naming the corresponding instance variable database_name is confusing. Finally, the comment in the old code about the database parameter is incorrect - if a parameter required, just don't put in a default value. Python still allows using a keyword for it in the method call, and the keyword parameters can be in any order. You'll get a failure if the required parameter is missing, which is the desired behavior.

(There are other issues as well, for example, why is there something about silage in opus_core, but we'll ignore that for the moment.)

If a variable is the name of an object, include name in the object's name, e.g. variable_name, to help distinguish it from a variable that contains the object itself, e.g. variable.