Page 1 of 1
Static methods -> bad idea?
Posted: Tue Feb 27, 2007 3:24 pm
by Begby
I use static methods for a lot of stuff, but some of you have said its a bad idea due to unit tests. I haven't done much with unit tests but will be implementing them on the newest project I am working on that is quite large.
Right now I use static methods like this example below
Code: Select all
// This returns an order collection object
$orders = FCP_Model_Order::getOpenOrdersForClient($clientID) ;
// get a single order
$order = FCP_Model_Order::find($orderID) ;
// create a new order
$order = new FCP_Model_Order();
$order->buildFromXML( $xml ) ;
$order->insert() ;
If this is not correct, how is it best handled? Should I create a manager class? If so can the manager class call the static methods from FCP_Model_Orders or does all the sql need to go into the manager class as well?
Code: Select all
$orderMgr = new FCP_Model_OrderManager() ;
$orders = $orderMgr->getOpenOrdersForClient($clientID) ;
I also have a database class that is reference everywhere like the following
Code: Select all
// This returns a FCP_DB_PDO using the config uri and authentication info
$stmt = FCP_DB::instance()->prepare($sql) ;
// Its not a singleton though, we can also do this
$pdo = new FCP_DB_PDO( $uri, $login, $pass );
So I assume I can still do unit tests on the PDO object and also on any classes that use it even if they get it from the FCP_DB::instance() method (which is kinda like a mini registry)
Posted: Tue Feb 27, 2007 3:36 pm
by feyd
Static methods aren't as much a problem as Singletons for unit testing that I've seen.
Re: Static methods -> bad idea?
Posted: Tue Feb 27, 2007 3:56 pm
by Christopher
Begby wrote:Code: Select all
$orderMgr = new FCP_Model_OrderManager() ;
$orders = $orderMgr->getOpenOrdersForClient($clientID) ;
That's certainly clearer to me about what is responsible for what that your example of your current code which is a mix of OO and procedural using the same class. The Order Manager is a Gateway to the orders database and can find an Order, which is a single record -- perhaps Active Record.
Re: Static methods -> bad idea?
Posted: Tue Feb 27, 2007 4:05 pm
by Begby
arborint wrote:Begby wrote:Code: Select all
$orderMgr = new FCP_Model_OrderManager() ;
$orders = $orderMgr->getOpenOrdersForClient($clientID) ;
That's certainly clearer to me about what is responsible for what that your example of your current code which is a mix of OO and procedural using the same class. The Order Manager is a Gateway to the orders database and can find an Order, which is a single record -- perhaps Active Record.
I am inclined to agree. Also, as I think about this I can create different managers for different things, so if the other dude coding with me wants to cludge up my classes with some new query to return a specific collection that he wants, I can make him put it into his own manager class, thereby keeping my stuff pristine and pretty.
Re: Static methods -> bad idea?
Posted: Tue Feb 27, 2007 4:15 pm
by Christopher
Begby wrote:so if the other dude coding with me wants to cludge up my classes with some new query to return a specific collection that he wants, I can make him put it into his own manager class, thereby keeping my stuff pristine and pretty.
Separation of Concerns ... it takes your breath away!

Posted: Wed Feb 28, 2007 2:25 am
by CoderGoblin
Whilst I try to avoid static methods there are, in my opinion, times when it makes sense from a maintenance point of view. Example:
If I have a user class. functions such as login/register etc go in this class. I also have the possibility to have a function to check if a user exists. Having this as a static method means that I know where it is, as it is dealing with user processing although not for the current user. For other "common" functions which have no real logical connection to any classes I prefer to simply have an include file with functions in rather than as a class with static methods.
Posted: Wed Feb 28, 2007 12:15 pm
by Christopher
CoderGoblin wrote:For other "common" functions which have no real logical connection to any classes I prefer to simply have an include file with functions in rather than as a class with static methods.
Yes ... helper function are very handy and can simplify code. But you need the wisdom to know when to use them and when to use a real object. I should note that the only short term downside to using a real object is a little extra code. In the long term I have found that using a real object is usually better as complexity increases (and it always does

). But, being a OO purist in PHP is neither possible, nor the best idea.
Posted: Thu Mar 01, 2007 11:59 am
by Begby
Ok, here is what I did. I created managers for different types of classes that will be accessed with a variety of queries. The managers can be instantiated by themselves, but I also included them into a static registry type thing for ease of use.
Secondly I kind of nested the access to the managers based on the namespace, so some of the managers also contain a static registry of other managers.
One of the primary reasons for using managers in this way (with the static / nested registry) is that there will be a lot of use of these objects, so it will help with typing out all the pear names, and also I am trying to provide a really simple intuitive api to other programmers as I am the backend guy, and they do the views and controller stuff.
Here is what it looks like now. I find it a lot more intuitive than using the static methods like I had before, also now my model classes aren't cludged with a bunch of static methods.
Code: Select all
// Access FCP_Managers_OrderMgr and get an order based on an orderID
$order = FCP::orders()->find($orderID) ;
// Print the skus of the items on backorder
foreach( $order->backorders() as $backorder )
{
echo $backorder->getSKU() ;
}
// Get the shipment for an order using the orderID, this uses an instance of FCP_Managers_Order_ShipmentMgr
$shipment = FCP::orders()->shipments()->findByOrderID($orderID) ;
// Get a paged collection of products using a clientID and the product manager
$products = FCP::products()->getPageForClient($_GET['page'], $clientID) ;
echo $products->drawPaginateMenu() ;
foreach( $products as $product )
{
echo $product->getSKU() ;
}
Posted: Thu Mar 01, 2007 2:05 pm
by Christopher
I just changed the code around a little to maybe give you some ideas:
Code: Select all
// Access FCP_Managers_OrderMgr and get an order based on an orderID
$orders = FCP_Orders($db);
$products = FCP_Products($db) ;
$view= new FCP_View();
$content = '';
// Print the skus of the items on backorder
$backorder_skus = $orders->findBackorderSkus($orderID) ;
foreach($backorder_skus as $sku)
{
$content .= $view->drawBackorderSku($sku);
}
// Get the shipment for an order using the orderID, this uses an instance of FCP_Managers_Order_ShipmentMgr
$shipment = $orders->shipments()->findByOrderID($orderID) ;
$content .= $view->drawPaginateMenu() ;
// Get a paged collection of products using a clientID and the product manager
$clients_skus = $products->getPageOfSkusByClient($request->get('page'), $clientID) ;
foreach($clients_skus as $sku)
{
$content .= $view->drawSku($sku);
}
// gather output rather than echo so you can still start session or redirect as needed
$response->setContent($content);
Posted: Thu Mar 01, 2007 5:26 pm
by Chris Corbyn
Static methods shouldn't cause problems with unit tests... it's static class fields that do that. Static class fields are effectively globals and the reason heavy use of them is often seen as lazy programming. The only thing it makes difficult is mocking the class... now some may argue that that's not a problem with statics, but a problem with the workings of a mock object.
Personally, I use static methods sparingly but you'll probably see one or two in every two or three classes I write as factories.
Posted: Sun Mar 04, 2007 11:15 am
by Ollie Saunders
Object oriented purists will argue that static methods are procedural code. Without $this I can how they could have such an opinion.
The problem with statics is one of polymorphism. A static method is tied to a particular class where a normal one is only tied to an instance of an object, which can, of course, be constructed by any code. Unless you are going over the top with old type hinting the instance based method is going to allow you to swap out behaviour for something else where the static isn't.
You
can do this...
Code: Select all
call_user_func(array($class, 'message'));
...to get around the problem but its more effort, isn't as clear and probably slower than an object...