some small refactorings. adding docblocks. creating guard clauses. refactor our some code into methods. no aditional test failures so it looks ok. also added some @todo comments

lib/Doctrine/Access.php

@@ -103,6 +103,8 @@ abstract class Doctrine_Access extends Doctrine_Locator_Injectable implements Ar
     }
 
     /**
+     * Check if an offsetExists. Alias for contains.
+     *
      * @param mixed $offset
      * @return boolean          whether or not this object contains $offset
      */
@@ -113,6 +115,7 @@ abstract class Doctrine_Access extends Doctrine_Locator_Injectable implements Ar
 
     /**
      * offsetGet    an alias of get()
+     *
      * @see get,  __get
      * @param mixed $offset
      * @return mixed

lib/Doctrine/AuditLog.php

@@ -40,11 +40,24 @@ class Doctrine_AuditLog extends Doctrine_Plugin
                             'pluginTable'   => false,
                             );
 
+    /**
+     * Create a new auditlog_ 
+     * 
+     * @param array $options An array of options
+     * @return void
+     */
     public function __construct($options)
     {
         $this->_options = array_merge($this->_options, $options);
     }
 
+    /**
+     * Get the version 
+     * 
+     * @param Doctrine_Record $record 
+     * @param mixed $version 
+     * @return array An array with version information
+     */
     public function getVersion(Doctrine_Record $record, $version)
     {           
         $className = $this->_options['className'];
@@ -65,6 +78,13 @@ class Doctrine_AuditLog extends Doctrine_Plugin
 
         return $q->execute($values, Doctrine::HYDRATE_ARRAY);
     }
+
+    /**
+     * buildDefinition for a table 
+     * 
+     * @param Doctrine_Table $table 
+     * @return boolean true on success otherwise false.
+     */
     public function buildDefinition(Doctrine_Table $table)
     {
         $this->_options['className'] = str_replace('%CLASS%', 

lib/Doctrine/Cache.php

@@ -312,7 +312,9 @@ class Doctrine_Cache extends Doctrine_EventListener implements Countable, Iterat
 
         $data  = false;
         // only process SELECT statements
-        if (strtoupper(substr(ltrim($query), 0, 6)) == 'SELECT') {
+        if (strtoupper(substr(ltrim($query), 0, 6)) != 'SELECT') {
+            return false;
+        }
 
         $this->add($query, $event->getInvoker()->getName());
 
@@ -338,7 +340,6 @@ class Doctrine_Cache extends Doctrine_EventListener implements Countable, Iterat
             $this->_data = $data;
             return true;
         }
-        }
         return false;
     }
 
@@ -387,7 +388,9 @@ class Doctrine_Cache extends Doctrine_EventListener implements Countable, Iterat
         $data  = false;
 
         // only process SELECT statements
-        if (strtoupper(substr(ltrim($query), 0, 6)) == 'SELECT') {
+        if (strtoupper(substr(ltrim($query), 0, 6)) != 'SELECT') {
+            return false;
+        }
 
         $this->add($query, $event->getInvoker()->getDbh()->getName());
 
@@ -416,7 +419,6 @@ class Doctrine_Cache extends Doctrine_EventListener implements Countable, Iterat
             $this->_data = $data;
             return true;
         }
-        }
         return false;
     }
 }

lib/Doctrine/Compiler.php

@@ -26,8 +26,8 @@
  * @package     Doctrine
  * @subpackage  Compiler
  * @author      Konsta Vesterinen <kvesteri@cc.hut.fi>
- * @license     http://www.opensource.org/licenses/lgpl-license.php LGPL
+ * @license     http://www.opensource.org/licenses/lgpllicense.php LGPL
- * @link        www.phpdoctrine.com
+ * @link        www.phpdoctrine.
  * @since       1.0
  * @version     $Revision$
  */
@@ -69,6 +69,8 @@ class Doctrine_Compiler
         foreach ($it as $file) {
             $e = explode('.', $file->getFileName());
             
+            //@todo what is a versioning file? do we have these anymore? None 
+            //exists in my version of doctrine from svn.
             // we don't want to require versioning files
             if (end($e) === 'php' && strpos($file->getFileName(), '.inc') === false) {
                 require_once $file->getPathName();

lib/Doctrine/Configurable.php

@@ -379,14 +379,15 @@ abstract class Doctrine_Configurable extends Doctrine_Locator_Injectable
             throw new Doctrine_Exception('Unknown attribute.');
         }
 
-        if ( ! isset($this->attributes[$attribute])) {
+        if (isset($this->attributes[$attribute])) {
+            return $this->attributes[$attribute];
+        }
+        
         if (isset($this->parent)) {
             return $this->parent->getAttribute($attribute);
         }
         return null;
     }
-        return $this->attributes[$attribute];
-    }
 
     /**
      * getAttributes

lib/Doctrine/Connection.php

@@ -262,6 +262,9 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * setAttribute
      * sets an attribute
      *
+     * @todo why check for >= 100? has this any special meaning when creating 
+     * attributes?
+     *
      * @param integer $attribute
      * @param mixed $value
      * @return boolean
@@ -470,9 +473,6 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      */
     public function replace(Doctrine_Table $table, array $fields, array $keys)
     {
-        //if ( ! $this->supports('replace'))
-        //    throw new Doctrine_Connection_Exception('replace query is not supported');
-
         if (empty($keys)) {
             throw new Doctrine_Connection_Exception('Not specified which fields are keys');
         }

lib/Doctrine/Expression.php

@@ -36,6 +36,13 @@ class Doctrine_Expression
     
     protected $_conn;
 
+    /**
+     * Create an expression 
+     * 
+     * @param string $expr The expression
+     * @param Doctrine_Connection $conn The connection (optional)
+     * @return void
+     */
     public function __construct($expr, $conn = null)
     {
         $this->setExpression($expr);
@@ -45,6 +52,11 @@ class Doctrine_Expression
         }
     }
 
+    /**
+     * getConnection 
+     * 
+     * @return Doctrine_Connection The connection
+     */
     public function getConnection()
     {
         if ( ! isset($this->_conn)) {
@@ -54,11 +66,25 @@ class Doctrine_Expression
         return $this->_conn;
     }
 
+    /**
+     * setExpression 
+     * 
+     * @param string $clause The expression to set
+     * @return void
+     */
     public function setExpression($clause)
     {
         $this->_expression = $this->parseClause($clause);
     }
 
+    /**
+     * parseExpression 
+     *
+     * @todo: What does this function do?
+     * 
+     * @param string $expr The expression to parse
+     * @return void
+     */
     public function parseExpression($expr)
     {
         $pos  = strpos($expr, '(');
@@ -78,6 +104,12 @@ class Doctrine_Expression
         return call_user_func_array(array($this->getConnection()->expression, $name), $args);
     }
 
+    /**
+     * parseClause 
+     * 
+     * @param string $clause The clause
+     * @return string The parse clause
+     */
     public function parseClause($clause)
     {
         $e = Doctrine_Tokenizer::bracketExplode($clause, ' ');
@@ -89,12 +121,22 @@ class Doctrine_Expression
         return implode(' ', $e);
     }
 
+    /**
+     * getSql 
+     * 
+     * @return string The expression
+     */
     public function getSql()
     {
 
         return $this->_expression;
     }
 
+    /**
+     * __toString 
+     * 
+     * @return void
+     */
     public function __toString()
     {
         return $this->getSql();

lib/Doctrine/Formatter.php

@@ -46,7 +46,9 @@ class Doctrine_Formatter extends Doctrine_Connection_Module
      */
     public function escapePattern($text)
     {
-        if ($this->string_quoting['escape_pattern']) {
+        if ( ! $this->string_quoting['escape_pattern']) {
+            return $text;
+        }
         $tmp = $this->conn->string_quoting;
 
         $text = str_replace($tmp['escape_pattern'], 
@@ -56,7 +58,6 @@ class Doctrine_Formatter extends Doctrine_Connection_Module
         foreach ($this->wildcards as $wildcard) {
             $text = str_replace($wildcard, $tmp['escape_pattern'] . $wildcard, $text);
         }
-        }
         return $text;
     }
 

lib/Doctrine/Hook.php

@@ -208,7 +208,10 @@ class Doctrine_Hook
     }
 
     /**
+     * set the hook limit 
+     * 
      * @param integer $limit 
+     * @return void
      */
     public function hookLimit($limit)
     {
@@ -216,6 +219,8 @@ class Doctrine_Hook
     }
 
     /**
+     * set the hook offset
+     *
      * @param integer $offset
      */
     public function hookOffset($offset)

lib/Doctrine/Hydrate.php

@@ -254,15 +254,10 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
      */
     public function useCache($driver = true, $timeToLive = null)
     {
-        if ($driver !== null) {
+        if($driver !== null && $driver !== true && !($driver instanceOf Doctrine_Cache_Interface)){
-            if ($driver !== true) {
-                if ( ! ($driver instanceof Doctrine_Cache_Interface)) {
             $msg = 'First argument should be instance of Doctrine_Cache_Interface or null.';
-
             throw new Doctrine_Hydrate_Exception($msg);
         }
-            }
-        }
         $this->_cache = $driver;
 
         return $this->setCacheLifeSpan($timeToLive);
@@ -549,15 +544,15 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
      */
     public function removeQueryPart($name)
     {
-        if (isset($this->parts[$name])) {
+        if ( ! isset($this->parts[$name])) {
+            throw new Doctrine_Hydrate_Exception('Unknown query part ' . $name);
+        }
+
         if ($name == 'limit' || $name == 'offset') {
                 $this->parts[$name] = false;
         } else {
                 $this->parts[$name] = array();
         }
-        } else {
-            throw new Doctrine_Hydrate_Exception('Unknown query part ' . $name);
-        }
         return $this;
     }
 
@@ -798,6 +793,13 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
 
         return serialize(array($resultSet, $map, $this->getTableAliases()));
     }
+
+    /**
+     * _execute 
+     * 
+     * @param array $params 
+     * @return void
+     */
     public function _execute($params)
     {
         $params = $this->_conn->convertBooleans($params);
@@ -1019,6 +1021,9 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
      * The key idea is the loop over the rowset only once doing all the needed operations
      * within this massive loop.
      *
+     * @todo: Can we refactor this function so that it is not so long and 
+     * nested?
+     *
      * @param mixed $stmt
      * @return array
      */
@@ -1240,7 +1245,8 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
         }
         if ($index !== false) {
             $prev[$alias] =& $coll[$index];
-        } else {
+            return;
+        }
         // first check the count (we do not want to get the last element
         // of an empty collection/array)
         if (count($coll) > 0) {
@@ -1260,7 +1266,6 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
             }
         }
     }
-    }
 
     /**
      * @return string                   returns a string representation of this object

lib/Doctrine/IntegrityMapper.php

@@ -32,12 +32,26 @@
  */
 class Doctrine_IntegrityMapper 
 {
+    /**
+     * processDeleteIntegrity 
+     * 
+     * @param Doctrine_Record $record 
+     * @return void
+     */
     public function processDeleteIntegrity(Doctrine_Record $record)
     {
         $coll = $this->buildIntegrityRelationQuery($record);
         
         $this->invokeIntegrityActions($record);
     }
+
+
+    /**
+     * invokeIntegrityActions 
+     * 
+     * @param Doctrine_Record $record 
+     * @return void
+     */
     public function invokeIntegrityActions(Doctrine_Record $record)
     {
         $deleteActions = Doctrine_Manager::getInstance()
@@ -65,6 +79,13 @@ class Doctrine_IntegrityMapper
             }
         }
     }
+
+    /**
+     * buildIntegrityRelationQuery 
+     * 
+     * @param Doctrine_Record $record 
+     * @return array The result
+     */
     public function buildIntegrityRelationQuery(Doctrine_Record $record)
     {
         $q = new Doctrine_Query();
@@ -95,6 +116,17 @@ class Doctrine_IntegrityMapper
 
         return $q->execute(array($params));
     }
+
+    /**
+     * buildIntegrityRelations 
+     * 
+     * @param Doctrine_Table $table 
+     * @param mixed $aliases 
+     * @param mixed $fields 
+     * @param mixed $indexes 
+     * @param mixed $components 
+     * @return void
+     */
     public function buildIntegrityRelations(Doctrine_Table $table, &$aliases, &$fields, &$indexes, &$components)
     {
         $deleteActions = Doctrine_Manager::getInstance()

lib/Doctrine/Lib.php

@@ -221,7 +221,11 @@ class Doctrine_Lib
     }
 
     /**
-     * @return string
+     * formatSql 
+     * 
+     * @todo: What about creating a config varialbe for the color?
+     * @param mixed $sql 
+     * @return string the formated sql
      */
     public static function formatSql($sql)
     {

lib/Doctrine/Manager.php

@@ -82,6 +82,7 @@ class Doctrine_Manager extends Doctrine_Configurable implements Countable, Itera
      * setDefaultAttributes
      * sets default attributes
      *
+     * @todo I do not understand the flow here. Explain or refactor?
      * @return boolean
      */
     public function setDefaultAttributes()
@@ -317,6 +318,12 @@ class Doctrine_Manager extends Doctrine_Configurable implements Countable, Itera
         return $this->_connections[$name];
     }
     
+    /**
+     * parsePdoDsn 
+     * 
+     * @param array $dsn An array of dsn information 
+     * @return array The array parsed
+     */
     public function parsePdoDsn($dsn)
     {
         $parts = array();

lib/Doctrine/Migration.php

@@ -114,22 +114,15 @@ class Doctrine_Migration
         }
     }
 
+
     /**
-     * loadMigrationClasses
+     * loadMigrationClassesFromDirectory 
-     *
-     * Loads the migration classes for the directory specified by the constructor
      * 
+     * refactored out from loadMigrationClasses
+     * $param array An array of classes
      * @return void
      */
-    protected function loadMigrationClasses()
+    public function loadMigrationClassesFromDirectory($classes){
-    {
-        if ($this->_migrationClasses) {
-            return $this->_migrationClasses;
-        }
-        
-        $classes = get_declared_classes();
-        
-        if ($this->_migrationClassesDirectory !== null) {
         foreach ((array) $this->_migrationClassesDirectory as $dir) {
             $it = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir),
                 RecursiveIteratorIterator::LEAVES_ONLY);
@@ -152,6 +145,26 @@ class Doctrine_Migration
         }
     }
 
+    /**
+     * loadMigrationClasses
+     *
+     * Loads the migration classes for the directory specified by the constructor
+     *
+     * @return void
+     */
+    protected function loadMigrationClasses()
+    {
+        if ($this->_migrationClasses) {
+            return $this->_migrationClasses;
+        }
+        
+        $classes = get_declared_classes();
+        
+        if ($this->_migrationClassesDirectory !== null) {
+            $this->loadMigrationClassesFromDirectory($classes);
+        }
+
+        
         $parent = new ReflectionClass('Doctrine_Migration');
         
         foreach ($this->_loadedMigrations as $name => $fileName) {
@@ -317,7 +330,9 @@ class Doctrine_Migration
      */
     protected function doMigrate($direction)
     {
-        if (method_exists($this, $direction)) {
+        if (! method_exists($this, $direction)) {
+            return;
+        }
         $this->$direction();
 
         foreach ($this->_changes as $type => $changes) {
@@ -329,7 +344,6 @@ class Doctrine_Migration
             }
         }
     }
-    }
 
     /**
      * migrate

lib/Doctrine/Search.php

@@ -49,6 +49,12 @@ class Doctrine_Search extends Doctrine_Plugin
     protected $_built = false;
 
     
+    /**
+     * __construct 
+     * 
+     * @param array $options 
+     * @return void
+     */
     public function __construct(array $options)
     {
         $this->_options = array_merge($this->_options, $options);
@@ -62,6 +68,12 @@ class Doctrine_Search extends Doctrine_Plugin
     }
 
 
+    /**
+     * search 
+     * 
+     * @param string $query 
+     * @return Doctrine_Collection The collection of search results
+     */
     public function search($query)
     {
         $q = new Doctrine_Search_Query($this->_options['pluginTable']);
@@ -71,6 +83,12 @@ class Doctrine_Search extends Doctrine_Plugin
         return $this->_options['connection']->fetchAll($q->getSql(), $q->getParams());;
     }
     
+    /**
+     * analyze 
+     * 
+     * @param string $text 
+     * @return void
+     */
     public function analyze($text)
     {
         return $this->_options['analyzer']->analyze($text);
@@ -131,6 +149,13 @@ class Doctrine_Search extends Doctrine_Plugin
         }
     }
 
+    /**
+     * readTableData 
+     * 
+     * @param mixed $limit 
+     * @param mixed $offset 
+     * @return Doctrine_Collection The collection of results
+     */
     public function readTableData($limit = null, $offset = null)
     {
         $this->buildDefinition(); 
@@ -152,6 +177,13 @@ class Doctrine_Search extends Doctrine_Plugin
     
 
 
+    /**
+     * batchUpdateIndex 
+     * 
+     * @param mixed $limit 
+     * @param mixed $offset 
+     * @return void
+     */
     public function batchUpdateIndex($limit = null, $offset = null)
     {
         $this->buildDefinition();
@@ -202,6 +234,11 @@ class Doctrine_Search extends Doctrine_Plugin
         }
     }
 
+    /**
+     * buildDefinition 
+     * 
+     * @return void
+     */
     public function buildDefinition()
     {
     	if ($this->_built) {

lib/Doctrine/Template.php

@@ -85,6 +85,14 @@ class Doctrine_Template extends Doctrine_Record_Abstract
         return $this->_invoker;
     }
 
+    /**
+     * addChild 
+     *
+     * Adds a plugin as a child to this plugin
+     * 
+     * @param Doctrine_Template $template 
+     * @return Doctrine_Template. Chainable.
+     */
     public function addChild(Doctrine_Template $template)
     {
         $this->_plugin->addChild($template);
@@ -93,24 +101,53 @@ class Doctrine_Template extends Doctrine_Record_Abstract
     }
 
 
+    /**
+     * getPlugin 
+     * 
+     * @return void
+     */
     public function getPlugin()
     {
         return $this->_plugin;
     }
 
+    /**
+     * get 
+     * 
+     * @param mixed $name 
+     * @return void
+     */
     public function get($name) 
     {
         throw new Doctrine_Exception("Templates doesn't support accessors.");
     }
+
+    /**
+     * set 
+     * 
+     * @param mixed $name 
+     * @param mixed $value 
+     * @return void
+     */
     public function set($name, $value)
     {
         throw new Doctrine_Exception("Templates doesn't support accessors.");
     }
+    /**
+     * setUp 
+     * 
+     * @return void
+     */
     public function setUp()
     {
 
     }
 
+    /**
+     * setTableDefinition 
+     * 
+     * @return void
+     */
     public function setTableDefinition()
     {