How Many Static Methods is Too Many For One Class?

Question

UPDATE: Rephrasing the question to ask, 'are there too many' static methods (I realize that right now there are only 4 but I originally started with 2) in this class structure? If so, any suggestions on how to refactor these classes to use some sort of Finder class so that I can remove the static functions from the Model classes?

I have the following abstract class:

abstract class LP_Model_Abstract
{
protected static $_collectionClass = 'LP_Model_Collection';

protected $_row = null;

protected $_data = array();

public function __construct($row = null)
{
    $this->_row = $row;
}

public function __get($key)
{
    if(method_exists($this, '_get' . ucfirst($key)))
    {
        $method = '_get' . ucfirst($key);
        return $this->$method();            
    }
    elseif(isset($this->_row->$key))
    {
        return $this->_row->$key;
    }
    else
    {
        foreach($this->_data as $gateway)
        {
            if(isset($gateway->$key))
            {
                return $gateway->$key;
            }
        }
    }   
}

public function __set($key, $val)
{
    if(method_exists($this, '_set' . ucfirst($key)))
    {
        $method = '_set' . ucfirst($key);
        return $this->$method($val);            
    }
    elseif(isset($this->_row->$key))
    {
        $this->_row->$key = $val;
        return $this->_row->$key;
    }
    else
    {
        foreach($this->_data as $gateway)
        {
            if(isset($this->_data[$gateway]->$key))
            {
                $this->_data[$gateway]->$key = $val;
                return $this->_data[$gateway]->$key;
            }
        }
    }
}

public function __isset($key)
{
    return isset($this->_row->$key);
}

public function save()
{
    $this->_row->save();
}

abstract public static function get($params);
abstract public static function getCollection($params = null);
abstract public static function create($params);

}

And then this class which provides additional functionality for class table inheritance schemes (where type is important in determining additional functionality in a factory fashion):

abstract class LP_Model_Factory_Abstract extends LP_Model_Abstract
{
    protected static $_collectionClass = 'LP_Model_Collection_Factory';

    abstract public static function factory($row);
}

These ultimately result in the following type of class declaration:

class Model_Artifact extends LP_Model_Factory_Abstract
{
    protected static $_artifactGateway = 'Model_Table_Artifact';

    public static function create($params)
    {

    }

    public static function get($params) 
    {
        $gateway = new self::$_artifactGateway();

        $row = $gateway->fetchArtifact($params);

        return self::factory($row);        
    }

    public static function getCollection($params = null) 
    {
        $gateway = new self::$_artifactGateway();

        $rowset = $gateway->fetchArtifacts($params);

        $data = array(
            'data' => $rowset,
            'modelClass' => __CLASS__
        );

        return new self::$_collectionClass($data);
    }

    public static function factory($row)
    {
        $class = 'Model_Artifact_' . $row->fileType;
    }
}

When do you know that you have too many static methods in a class? And how would you refactor the existing design so that the static methods are perhaps encapsulated in some sort of Finder class?

Answer 1

If you want to build reusable and testable code, you should avoid static methods altogether. Code which calls static methods (or constructors of non-data-like classes) cannot be tested in isolation.

Yes, you will have to pass around alot more objects if you eliminate static methods. This is not necessarily a bad thing. It forces you to think about the boundaries and cooperation between your components in a disciplined way.

Answer 2

I agree with BaileyP and I'll add my couple of pennies:

I always work with the idea that a class should have a single reason for existing; it should have one job that it does, and it should do it well. After deciding that, and figuring out what the interface to that class should be, I go through and mark any functions that don't change the state of an instance of the class as static.

Answer 3

Personally I find that any number of static methods are a sign of trouble. If your class has instance methods and static methods, then most likely you could split the class into two separate entities and change the static methods to instance methods.

Think of a class as a special kind of object, with the distinctive property that it is global by nature. Since it's a global variable, it implies a very strong level of coupling, so you would want to reduce any references to it. Static members will need to be referred, meaning that your code will get a strong level of coupling to the class.

Answer 4

I'll throw in my 2 cents.

First of all, I'll agree that setting some sort of arbitrary limit is not helpful, such as "Once I have more than 10 statics in a class that's too many!". Refactor when it makes sense but don't start doing it just because you've hit some imaginary boundary.

I wouldn't 100% agree with Brubaker's comment about stateful vs. stateless - I think the issue is more about classes vs instances. Because a static method can change the value of another static property which is a stateful change.

So, think of it like this - if the method/property is of or pertaining to the class, then it should probably be static. If the method/property is of or pertaining to an instance of the class, it should not be static.

Answer 5

I'd have to agree with Brubaker and add that to my thinking it isn't the number of methods so much as the functionality of said methods. If you start thinking that your class has to many methods (static or otherwise) then you might find they can be re-grouped and refactored into a more intuitive architecture.

Answer 6

The first indicator I use when determining if I have to many static methods is if the methods functionality is not stateless. If the static methods change the state of the object they reside in, they probably shouldn't be static.