4

In my example I'm using the PHP framework Yii2 but I think this applies to most OO languages.

I have an ActiveRecord base class which most of my business objects extend from e.g. Project.

At the moment if I want a Project instance I call

Project::findOne(['id' => $id]);

findOne is a static method of ActiveRecord (which is part of the Yii2 framework). So this is bad form because I can't easily mock/stub the return of this call when writing unit tests.

But what's the best way to get around this?

I could create a class CActiveRecord that inherits from ActiveRecord and wrap the static call in a non-static call and use that everywhere - but then I would have to instantiate a throw-away Project object in order to get the actual instance. What if the Project object needed some heavy config to be instantiated - I would be passing random nonsense into the constructor just to get an instance.

Summary: Simply changing statics to non-statics seems wrong - shouldn't I also move the functions somewhere else? If so, where?

Force Hero
  • 2,674
  • 3
  • 19
  • 35
  • Try searching for PHP LSB(Late Static Bindings). – Sotiris Kiritsis Oct 29 '15 at 08:45
  • Google global state. Also makes your code hard to test. The alternative is instantiating your objects and passing them around via dependency injection. Only codebases with poor architecture require statics. Ignore those who say there's a place for everything, like `goto` because there's one less opcode. – Jimbo Oct 29 '15 at 08:46
  • Also, active record is an incredibly naive pattern that couples your domain to its persistence. Take a look at DataMapper. – Jimbo Oct 29 '15 at 08:47
  • While this indeed might be a bad practice, it sounds like you chose your framework, and then you should just go with the framework (or find a better library). – Gerry Oct 29 '15 at 08:47
  • 1
    @Gerry The issue is that many frameworks impose a structure on you that's really pretty bad and will eventually come around to bite you in the arse when it's too late. You at least have to truly understand the tradeoffs you're agreeing to when "going with the framework" and decide whether it's worth it (e.g. because the project is and will remain "small", the framework fits the intended use perfectly and reduces the work of weeks to a few hours of generating your models or such). – deceze Oct 29 '15 at 08:53

1 Answers1

9

The issue with static calls is the hard coupling to a specific other piece of code. Just wrapping that in a "dynamic" call doesn't make this any better:

$c = new CProject;
$c->findOne(); // Calls Project::findOne()

That's pretty darn pointless. The issue is not the syntax of -> vs. ::, the issue is that this particular code references a specific other class and that you cannot easily exchange this class for something else. You're building rigid, hardcoded dependencies between your classes/objects, which makes it hard to take them apart, which makes your code hard to test, and which makes it harder to adapt code to different situations.

The alternative is dependency injection:

function foo(Project $project) {
    $p = $project->findOne();
}

This function is not coupled to any one specific Project class, but to a class which simply offers an interface akin to Project. In fact, Project could even be simply an interface. Which specific class and method is getting called here then is decided somewhere completely different, like your dependency injection container; or simply the caller of this code.

This makes it a lot easier to take this code apart and put it back together in different ways, as necessary for the situation at hand. That's not to say it can't work and that you should never use static calls at all, but you really need to be aware of what cross-dependencies you're establishing with every hardcoded class name, and whether that may or may not cause a problem down the line. For even moderately complex and/or growing software projects, it will almost certainly cause friction in some form or another eventually.

See How Not To Kill Your Testability Using Statics for a longer in-depth article.

deceze
  • 510,633
  • 85
  • 743
  • 889
  • Thanks for this, I've read a lot of articles recently that agree with what you're saying - and I also agree. What I don't quite understand is the best practise for this. I get DI but in your example the thing calling foo() would have to instantiate a throw away Project object - that doesn't seem right? Do you just create an "ActiveRecordHelper" or something that is lightweight to instantiate? – Force Hero Oct 29 '15 at 08:56
  • 2
    ActiveRecord is a problem in itself. If `Project` is both an object representing *an instance* of your data, and also the database connector itself, there's virtually no good way to handle this. At least you should be using a *Factory* then. `$projectFactory->findOne()` returns a `Project` instance. `ProjectFactory` itself would be instantiated like `new ProjectFactory($database)`. `$database` is an instance of `PDO` or whatever. You'd instantiate `ProjectFactory` exactly once and pass it to every function/class that will need to retrieve `Project` objects. Do you see the difference? – deceze Oct 29 '15 at 09:00
  • You need to think dependency injection through all the way to the "top". All dependencies are injected into everything. "The caller" may also just get other objects injected instead of instantiating them itself. You also need to separate responsibilities. Don't merge the responsibilities of getting data from the database and representing a record from the database into the same class. Managing this DI "all the way from the top" does introduce more complexity. That's where DI containers come in. Play around with Symfony for a change, their DI stuff is pretty good and shows it in practice. – deceze Oct 29 '15 at 09:04
  • Ok cool, I'm kind of almost there... If I instantiate ProjectFactory only once doesn't that mean that the entry script (say a controller action) needs to know that 5 calls down something needs a ProjectFactory? That would end up with each entry script needing to know what everything that it calls needs and what they call needs etc... (clearly this isn't right - I'm being dumb... ) – Force Hero Oct 29 '15 at 09:16
  • 1
    Yes, exactly, you got it. That's the additional complexity I'm talking about. Depending on your particular objects, it's manageable and/or desirable for the "parent" object to know its dependencies. But again, that's also where DI containers or "Service Locators" come in. `$diContainer->get('ProjectFactory')`, where `$diContainer` is configured to know what dependencies `ProjectFactory` needs and can construct the object with all dependencies resolved in a deduplicated way. See http://symfony.com/doc/current/components/dependency_injection/introduction.html – deceze Oct 29 '15 at 09:22
  • Ok, so the "default" dependencies for a class are setup by the container but things creating that class can override those "defaults" if they want to (using `$diContainer->get()` )? So this saves us writing the same dependencies every time we want a certain object? – Force Hero Oct 29 '15 at 09:26
  • 1
    "Overriding" the diContainer doesn't really enter the picture. If you have a class which wants to instantiate another object, and that object requires two dependencies, and those two dependencies may each require two more dependencies... then it's pretty tough for your class to have all these recursive dependencies and instantiate the object. That's the problem the diContainer solves. The class just has a diContainer (injected as dependency) and asks the diContainer to do all the work of resolving dependencies. If you need to customise instantiating, you reconfigure the diContainer. – deceze Oct 29 '15 at 09:30
  • That's what I meant, you just worded it better. :) Thanks a lot, big help!! – Force Hero Oct 29 '15 at 09:38
  • 2
    Think about it like this: each class you write is completely independent of other classes. You never directly reference any other particular class. At most you're doing some type hints; preferably those type hints are just for *interfaces*, not specific other classes. Now you have a bunch of completely independent classes which can be put together like Lego blocks. You now need to very specifically and carefully choose at which points those Lego blocks go together. A DI Container is one such centrally configured location, and it can easily be altered or replaced. – deceze Oct 29 '15 at 09:41
  • Also, don't use a Service Locator. It's an anti-pattern for a reason. – Jimbo Oct 29 '15 at 10:10