Blog post draft

The ins and outs of Global State.md

Earlier this month, I was asked by a colleague to explain why global state is bad. It seems to me that many developers don't fully understand what global state is, or if they do, why it is labelled as negative.

This post aims to show why global state is harmful, and how you can avoid it within your code. The examples are given in PHP, but you can apply the concepts to any object-oriented language.

Here's something worth distinguishing before we continue: calling a stateless static method isn't global state, it's just tight coupling. Even though these are two different issues, they're both harmful and avoidable.

What is Global State?

So what is global state?

Global state is essentially the same thing as insanity in this definition: a way to affect the execution of code hidden from sight, so that two apparently identical lines actually produce a different result depending on some external factor.

Source: Google chosen result

While working on a codebase, we may need access to utilities or 'helper objects' in multiple places throughout the application, or even objects that we expect to use "everywhere" like Database or Logger objects.

The global keyword provides access to variables defined within the global scope - which is to say that they are accessible globally from anywhere in the application.

Most developers know now not to use global within their code:

class MyObject
{
    public function doSomething()
    {
        /** Access objects from the global space **/
        global $logger;
        global $database;
    }
}

Instead, developers now believe they're playing it safe by using static calls to retrieve these objects intead:

/** Well, it's in a class, it MUST be safe... **/
class Resources
{
    public static $database;
    public static $logger;
}

/** Here's our object that uses the db and logger **/
class MyObject
{
    public function doSomething()
    {
        Resources::$database->query(/** Get some data **/);
        Resources::$logger->debug(/** Log all the things **/);
    }
}

The above code is effectively the same as using global. But why is this bad? Because anyone, or any object, can change this state at any time, and therefore you can't rely on it having the state you expect in your own objects.

Why is this important? When you're working in a team of more than one person, it's imperitive that every tool (object) you are using has a reliable state you can safely expect to work with every time you use that object.

When you open an object to the world (i.e. the rest of your codebase), anything can alter it at any time. If one object triggers an unexpected side-effect within the global state, you can no longer rely on the starting state of that object throughout all of it's uses in your codebase. Therefore, the object is inherently untestable.

This may not seem like a big issue in small codebases, but as soon as more than one developer gets involved, the problems become apparent. This isn't the only issue with global state.

Testibility

Inherently untestable code is bad code. It doesn't actually matter whether or not you write unit tests with 100% coverage - if your code is testable, then it's likely much better than code that is not.

Using statics or calls to external objects at most hooks you into the global state so you can't re-use the code easily anywhere else, and at least hooks you into the framework you're using so you are actually unable to take your code anywhere else.

Readability

Using global state or static calls introduces a class dependency that isn't explicitly announced in your codebase - meaning that future developers will have to go off and find this newly introduced class to figure out where it came from and why you're randomly pulling it in half-way through your code.

What an object needs to be built and to function should be available from that object's constructor / method signatures - including any primitives, objects or interfaces that are typehinted for. Simply making static calls in random places within a file means more work for future developers who will have to read your code line-by-line just to see if there are any dependencies required by the object they are looking at to make it work.

This might be okay for you because you know exactly why you're doing adding static calls and what the objects you are pulling in are used for. However, the next developer with the responsibility of maintaining all of your code will hate you, and you won't be able to easily use this code anywhere else. You can also forget about open-sourcing it as a standalone library.

Coupling

As a result of using a static call, you have now coupled the object you're coding to the object you're pulling in with the static methods available within it. This means that to use this code anywhere else, you're going to have to make sure that the static object you're calling exists, possibly in the exact same location, for the rest of time.

What's the problem with that? You can't use your object in any other way than the way you're currently designing. For example, if you couple your database abstraction to your custom logger by making static calls to that logger everywhere, you can't use that database class in another project without also copying over what might be an outdated and terribly written logger or heavily modifying it - which means it isn't re-usable.

Good developers dont write legacy code so don't make your code legacy right from the beginning by coupling your objects with static calls.

How to decouple

Dependency injection is a software design pattern that implements inversion of control and allows a program design to follow the dependency inversion principle. The term was coined by Martin Fowler. An injection is the passing of a dependency (a service) to a dependent object (a client).

Source: Wikipedia

Use Dependency Injection, which is the technical term for basically passing in objects as constructor or method parameters into other objects, meaning that no business objects (ignoring factories / builders) are directly responsible for instantiating other objects.

class Database
{
    protected $logger;

    public function __construct(Logger $logger)
    {
        $this->logger = logger;
    }
}

Although I am an advocate of using the Decorator Pattern to add additional functionality to objects, such as logging, I will go into this in a future post. For now the point to take in is that passing objects into other objects is a good way of decoupling your code.

Conclusion

Global state ensures you are writing Legacy code. If you hook multiple objects together with global state, your architecture is brittle, difficult to maintain, a nightmare for future developers and can make it nearly impossible for object re-use, never mind testability.

Don't make static calls. Don't use Laravel's "façades" (which are completely different from the actual Façade Pattern that hook you into using the same framework for the rest of time. Don't listen to those who try and convince you that global state is occasionally okay.

Be responsible with your codebase and put in the initial effort of avoiding global state to make refactoring a significantly easier thing to do in the future.

You might be conflating state and methods. Scala - a popular no-global-state culture still has singleton/static objects.

"Use dependency injection" is a cop-out. People love to say it (myself included), but it involves pretty significant changes that we can't just handwave away. For one, it all but requires builders and factories, for just about everything. That added complexity is not always justifiable when viewed from outside of the "global === evil" camp.

Hmm.. I don't see how it's a cop-out? It doesn't matter if you're using a framework or legacy code, at least write your objects for your own new parts / modules with best practice, so that they use DI, builders, factories etc. Then hook it into the legacy code via whatever bad practice they use (service locator, new calls everywhere) if you can't refactor it. As long as your module is flexible and extensible, there is still a light at the end of tunnel.

What would you add to that part to make it more concise?

Concise isn't the issue. In fact, it's the opposite of the issue -- you're blithely saying "use dependency injection" as if it will solve all your problems, when in fact it introduces new ones. Your words mean much much more than you're letting on.

Consider:

class A {
    public function doStuff() {
    }
}

Now, say we want the doStuff method to log that it was called.

With a static log method, this is downright trivial.

class A {
    public function doStuff() {
        Application::log("In doStuff");
    }
}

With dependency injection, on the other hand, you have to thread that dependency through the app. Everywhere an A is created, a logger must also somehow be available. So deciding to log something is no longer just adding a line that calls a method. You're adding a property to the class, adding a constructor so you can inject it, and altering the builder/factory to pass it. And you'd better be using a builder/factory, or you have bigger problems -- you'd have to change every place that creates an A. (Or, you could change every place that calls doStuff(), but that could introduce even bigger dependency issues.)

We'll have to agree to disagree on whether always having objects that do nothing but create other objects is overengineering. (In PHP, i believe it very often is.) But even if you believe it isn't, you still need to justify using dependency injection everywhere. It doesn't seem to justify itself here -- a log should be easy to write to from anywhere and everywhere, without a bunch of bureaucracy. What makes the additional complication of injecting it as a dependency better than just calling Application::log()?

Triviality is no excuse for poor code practices that prevent easy re-use, within or external to the framework you're using, or extension.

You can decorate your object and log in there for input and output logging, or use the observer pattern for logging business logic. The point is the logger isn't coupled to your object. I stated I'm more a fan of the decorator pattern for these sorts of things.

Calling Application::log() is a service locator for a start - a magic black box that contains anything and everything. I'm not about to go having other developers have to read through my code in order to see what my object needs to function - they will look at the constructor signature only. This is a pretty obvious requirement in the OO world...

Service Locator is an anti-pattern; it hides class dependencies, makes code more difficult to maintain and makes a liar of your API!

I'm a huge fan of a dependency injection container called Auryn. Once you set it up to automatically create your controllers, having any objects that you typehint for recursively instantiated and passed in for you, you don't even worry about these things you consider "overengineering". Create your factory, typehint for it, bam - you've got it. And you've got best practice and this simplicity you ask for. I really recommend giving it a look.

If you're interested, I gave a talk in Manchester, Knowsley and Majorca on Dependency Injection and best practice: Linky. There's also a fun lightning talk on there I think showing decorator and observer patterns in the context of logging.

PHP is no different from any other language - there are many, many ways to achieve good code with PHP now. It's nothing like it was 10 years ago, you should give it some more credit.

I'm well aware that there are ways to write good code with PHP. I actually prefer it over, say, Java. But i prefer it because it is different, and vastly so. It's a dynamically typed, multi-paradigm, single-threaded language, where the object graph is often built from scratch for each request. Which is pretty much the opposite of the statically typed wannabe-OO shared-everything thread-happy one-big-long-lived-app language that Java is. And those differences mean that a lot of what Java code has to jump through hoops to achieve, is built in in PHP.

With that said, DI for absolutely everything is typically overkill. Some things really should be available from everywhere. And if a global log affects testability, you're Doing It Wrong -- logging shouldn't be altering the functionality of the app in the first place.

DI for absolutely everything, when you have your tools set up correctly to allow this, means a hell of a lot less work - simply because your codebase is set up to allow this. You only need to add a string to your configuration to map abstract bases or interfaces to concrete objects and you have config-based polymorphism at your disposal with auto-wiring, saving so much time. There's nothing overkill about it when you have it set up - you don't need to make changes in the wrong places. Think JAVA Spring.

I'd rather not make exceptions for logging or anything for static calls. It's really simple to follow the rules - and just because I might prefer writing Static::Call() doesn't mean that the next developer who comes along will be happy with having to modify a load of code to use this somewhere else (which I have experienced many times). I'd rather make it much easier for that developer.

You're going to have to modify a load of something anyway. Configuration or code, makes little difference. Except that anything that you take in the constructor, you now require the caller to have available. Which requires that it be available in the caller, which requires it to be available in its caller, etc. DI containers are at best a workaround for that, and if you assume the other code is using one, then your code is just as broken as if you were relying on globals for everything.

More to the point, though...if everything is using it, and everything is passing it to everything else, guess what? You just have global state pretending not to be global state. You're often better off to just stop lying about it and use a global.

No. Everything doesn't use the container. Only your controllers, which from then onward you and other devs code forward from.

This way, you can create your SOLID OO code, write your tests, then plug it in as a component into your controller via a single extra typehint within either the controller constructor or 'action' method depending on how verbose you want to be with individual action requirements.

There's no extra modification needed except for interfaces / abstract classes to concretes in a configuration file. This can be a single key/value pair and that is it.

The caller for the object does not need to have it available to them - seeing as this is reflection-based recursive object instantiation, the objects are only created for the specific injection required. You shouldn't be passing objects through objects, just to pass them to another one further down the line (did you miss this when you researched DI, Auryn and Java Spring yourself?). If you are passing objects through to other objects, simply so another object further down the chain can use it, you need to seriously re-think your architecture.

The key point here that you seem to be missing is that doing it this way, you are no longer tied to the framework. Object requirements can be seen clearly from the constructor or method signature. Future engineers can pick up this component and re-use it somewhere else, be it in another library or completely separate project.

Adding static calls means devs can't pluck things out and use them elsewhere.

So, we've got to:

• Configuration for non-concrete injection can be a key:value pair in a format of your choice
• Everything else is automatically injected
• Code is decoupled and can be used in an entirely different place without the injector
• Your object API does what it should do - declare it's requirements for usage in it's signature

I suggest you come up with some objective reasons for your bias toward poor practice, which currently seems to be either laziness, a lack of understanding, or a chip on your shoulder. Either way, consider not saying someone is lying with their evidence if you want to be taken seriously.

I'm not saying you're lying to me. No, you're lying to yourself. For an object that's used by everything, DI does absolutely nothing to prevent the spooky action-at-a-distance that is the whole reason people discourage mutable global state. You're no worse off calling Application::log(). You can call it "poor practice" all you want, but the way you're recommending to "fix" it, is overengineered and very nearly worthless for the very example where you're pushing it.

You really want reasons?

• PHP code typically runs under an entirely different app model from Java. All that work you do to figure out how to even start up the application? It happens on every. single. request. That's anywhere from hundreds to literally billions of times as much as it does in an application that only starts once.and sticks around.
• If you're not writing a framework (and to some degree even if you are), YAGNI applies. Writing a quick little contact form or whatever, shouldn't require pulling in a bunch of crap. If you decide you want it to be reusable, then sure, go the DI route -- but keep in mind...
• Not all code needs to be reusable. That's a blatant lie sold to you by the Church of Object Oriented Everything. Fact is, pure, total, effortless reusability of all things doesn't exist, and chasing it is a classic cause of overengineering.
• Not all code should be object-oriented. PHP is a multi-paradigm language. OOP is just one of the tools in its box -- and frankly, is not the best for every case. There's procedural and FP as well, at least. And contrary to what the Church says, procedural code is not inherently "legacy" code or "bad" code, it's just different. I've seen -- and written -- decent amounts of procedural code that's leagues beyond what your typical OO weenie can churn out,
• No language feature is always bad -- if it were, it simply wouldn't exist. And people parroting the "Never ever ever use X" POV tend to be cargo-cultists and bandwagoners. Not particularly saying that's the case here, but your post does have no small amount of dogma to it.

Considering dependency injection is merely the (pointlessly) posh word for passing objects into other objects and therefore providing inversion of control, it's not at all over-engineering. It's absolutely the opposite. It's a simple change, providing you have your tools set up for it, that provides a lot of positives - some of which I've already explained.

• I'm fully aware PHP is not Java, I'm not sure this is an objective reason for not using DI.
• Re-usable code is something to aspire to provide for the future, regardless of whether or not it is possible in 100% of cases. I was never taught this, it is something I have picked up myself over the years through research and experience. I enjoy crafting re-usable code. I don't do it in 100% of cases, but I aim for it when designing systems when I can.
• I never stated all code should be object oriented. The rest of your paragraph on this point was unecessary.
• Language features exist and change with the times. Nobody uses goto any more because it was considered bad practice, yet the php interpreter actually uses one less opcode using a goto as opposed to a function call. I'm not actually sure why you pointed that out.

Call DI over-engineering all you like, but I will continue to provide code which helps future devs with reusability, with plenty of good comments, documentation and diagrams and good practice. And as I've heard many times, they're happier for it when it comes to modification or extension.

I didn't say DI itself was always overengineering. But it does require some infrastructure that doesn't always have to be there. You're saying it never is, and that globals are never useful. That is naïve to an extreme, and i've seen enough counterexamples of it to make the point look ridiculous.

Disagree if you want, it doesn't make the post look any less naïve. Or any less half-assed for providing a more complicated example that is actually easier in every way with globals.

Just to be clear, i'm not advocating "global everything". DI certainly can help simplify OO code tremendously, and it's hardly ever (actually, i could even agree with "never") a good idea to rely on global state that is both readable and writable. But there's a huge difference between sharing, say, a "database" (which could visibly and arbitrarily change state with each use) and a log file (which, by being effectively write-only, causes no changes that callers can base their behavior on) or an immutable data structure (which, by being effectively read-only, doesn't have the action-at-a-distance problem that mutables do). The unwillingness to accept that there actually are good use cases for globals, is IMO sabotaging your message.

There is always pretty much a better way than globals. Feel free to disagree while I write code that the next developer down the line has and will be happier to work with.

If you have the infrastructure in place to DI your database abstraction, you might as well DI your logger as well. I provided both as an example for what some people do with static calls to them. Either way, it's poor and bad for re-use. An extra class for logging some core business logic in a decorator is not a lot, at all.

I may /logger/database/s but the point still stands on statics and coupling.