4

Is it bad practice to have a lot of static functions? I am using Yii framework and I realized I have a lot of static functions in my model classes. I put all my functions that have to do with Users into the UsersModel (I do the same for other models too) but I end up with a lot of static functions. Just wondering how you guys deal with this. A lot of these functions are just query builder functions instead of lazy loading because I need to increase database performance.

Example functions:

User::getUserFromCampaign(1)
User::getUsersNotInCamapaigns()
User::isAdmin()
tereško
  • 58,060
  • 25
  • 98
  • 150
albertski
  • 2,428
  • 1
  • 25
  • 44
  • Probably this question should go to programmers.stackexchange – Fez Vrasta Feb 05 '14 at 18:08
  • Static functions make it extremely difficult to test: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html. – bblincoe Feb 05 '14 at 18:09
  • 2
    `static` methods are a pain to test, maintain, swap and have no place in proper OOP. So yeah I would say it is pretty bad practice. – PeeHaa Feb 05 '14 at 18:11

3 Answers3

1

Instead of using static functions, what you can do is instantiate an object of the User class (which would presumably not be a static class) in the models that require those methods, and use the object's methods directly.

This also means that those methods will only be loaded on objects that require them, instead of being "global".

Here is a good answer on this : https://softwareengineering.stackexchange.com/questions/98083/cant-i-just-use-all-static-methods

Community
  • 1
  • 1
Dany Caissy
  • 3,176
  • 15
  • 21
1

Instantiating your classes is best for testing but there is no problem using static for certain tasks.

A lot of it is down to opinion, if your code works, it's running efficiently and easy to maintain then all is dandy!

Also to add to the Laravel facade comments. Laravel does instantiate the class, a facade just provides a simplified interface to the bigger picture lets say.. that's exactly what laravel is doing. The end result is really nice readable code.

AJC
  • 69
  • 7
  • It is super readable... and very [robustly unit-testable, as well](http://laravel.com/docs/testing). – Leng Feb 05 '14 at 18:51
0

MVC is a nice design pattern and it has its place. The Factory design pattern is another nice pattern. In case you're unfamiliar with it, Google for Factory design pattern. In a few words: a FooFactory is a class that generates objects of the Foo class (or FooModel class, if you will).

MVC and Factory are not mutually exclusive, so you could refactor a lot of those static methods into a new UserFactoryClass.

  • User::getUserFromCampaign(1) to me, is a bit weird, though. I take it that 1 is a campaign ID? Then what user does it return? Or can a campaign only have one user? If that is the case, then UserFactory::getUserFromCampaign() will return a UserModel object for the user in the campaign with the given ID.

  • User::getUsersNotInCampaign(), I assume, returns an array of UserModel objects? Refactor it into `UserFactory::getUsersNotInCampaign() and there you go.

  • User::isAdmin() shouldn't be static at all. if ($user->isAdmin()) ..., not if(User::isAdmin($user))...

Andrejovich
  • 544
  • 2
  • 13
  • -1: please stop confusing "factory method" anti-pattern with proper factories. Also .. please flagging comments that you disagree with. – tereško Feb 06 '14 at 06:30
  • I didn't flag one single thing. In fact, your comment made me wonder about my apparent misinformation and I was hoping you could point me to resources about proper factories, the way they were ment. – Andrejovich Feb 06 '14 at 07:41
  • 1
    Well .. the first comment magically vanished (I just assumed that it was your doing, sorry for that mistake). Static factory method is considered an antipattern because of two issues. Immediate problem is caused by tight coupling, which is caused by use of static procedures. It also create a more long-term problem: if you start using inheritance in your codebase, the factory method becomes a "black hole of complexity", because it starts attracting large amounts of creational logic. – tereško Feb 06 '14 at 10:17
  • 2
    You should look into "Abstract Factory" and "Builder" patterns. There is also a pattern to which people refer to as *Concrete Factory". All of these patterns are based around the idea that (in adherence with [SRP](http://en.wikipedia.org/wiki/Single_responsibility_principle)) all of these approaches have a separate class instance, which is responsible for creation. This way you have an object, which you can be injected wherever you need it. – tereško Feb 06 '14 at 10:23