loading...

Static classes are evil

wrongabouteverything profile image wrong-about-everything Originally published at Medium ・3 min read

In spite of some languages, e.g., PHP, don’t have such thing as static classes, the concept is still present there. Class consisting entirely of static methods is effectively the same thing as static class.

Besides static classes are procedural and their clients are untestable (well, there are some hacks in Java and PHP, but I don’t even want to mention them), they have couple of more issues.

Classes tend to be big to huge

Since classes with static methods have nothing to do with objects, they don’t know who they are, what they should do and what they should not do. The boundaries are blurred, so we just write one instruction after another. It’s hard to stop until we’re done with our task. It is inevitably imperative and non-OOP process.

Dependencies are hidden

Code is way less readable. What’s in the class? Database query, some intense calculations, email sending? You just don’t control how many of them are in the class. One static method here, one there — and here it is, our new God object. And when you realize that, it’s already too late.

Low cohesion

Hence the class is getting less and less cohesive. If the class has a lot of dependencies, chances are that it does more than it should. For the sake of justice I should say that the possible reason of large number of dependencies is that they are at lower abstraction levels. So composing dependencies in higher-level abstractions could be the way to go.

Tight coupling

Static methods mean that they can be called from anywhere. They can be called from a lot of contexts. They have a lot of clients. So if one class needs some little special behavior to be implemented in static method, you need to make sure that none of the other clients got broken. So such reuse simply doesn’t work. I can compare it with noble (and failed) attempt to compose and reuse microservices. The resulting classes are too generic and completely unmaintainable. This results in the whole system being tightly coupled.

Example

As an example let’s consider client’s financial balance (taken from Payment Service Provider example). It has all mentioned drawbacks and looks like this:

class FinancialBalanceUtils
{
    static public function reserveBalance()
    {
        if (!self::shouldReserve()) {
            return;
        }
        self::assertAllConditionsAreMet();
        self::queryFinancialBalanceForCurrentClient();
        $result = self::checkFinancialBalance();
        if (!$result) {
            return false;
        }
        self::reserveFinancialBalanceForCurrentClient();
    }
}

So, let’s inspect it from the beginning.

We are not sure where it is called from as it can be called from absolutely anywhere. So we need to make sure that this method really should be executed, hence shouldReserve() method.
We are not sure where it is called from, once again! Are all preconditions satisfied? No one knows, so we absolutely must verify this —assertAllConditionsAreMet() method.
Then we get financial balance by tons of parameters from database with queryFinancialBalanceForCurrentClient() method, as the query and database table serve the needs of every client who needs financial balance.
OK, we are still not sure if the financial balance we got is fine. We need to check what’s in there — checkFinancialBalance().
Aaand finally we really do reserve the balance with reserveFinancialBalanceForCurrentClient().

I omitted logging, error handling and minor code quirks and kept only the essentials. And it’s already too big.
This class is a hidden dependency itself, and it consists of hidden dependencies. How much database queries are executed in this method? I don’t know.
Do you still have any doubts that this class does more than it should?
Was it worth it to allegedly avoid copy-paste which resulted in completely unmaintainable, super-generic code that is really scary to edit?

Try OOP instead

  • Identify your objects. Focus on “what”, not “how”. Focus on objects, not procedures.
  • When objects are identified, make all your dependencies explicit first, and limit their number. Limit it to five per method. Why five? I don’t know, it just sounds reasonable. Six feels too much already. When there are more than five probably your class wants to do more than it should. Or, probably, the abstraction level of those dependencies is too low. Anyway, explicit dependencies give you a chance to make your design more solid.
  • Don’t generalize too early, remember the Rule of Three. First implement only some specific scenarios.

Here is a cross-posted version from my medium post.

Discussion

markdown guide
 

I think the article is quite correct. But there is a critical unstated assumption: that static classes operate on a shared mutable state. The correct use of static methods are as deterministic functions. For example String.Join(",", strings) is probably deterministic in a given language, making it easily testable and understandable.

 

there is a critical unstated assumption: that static classes operate on a shared mutable state

There's no such assumption. Every issue I've listed holds true without it. And concerning your example, here is how I would write it:


interface String
{
    public function value();
}

class DefaultString implements String
{
    private $string;

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

    public function value()
    {
        return $this->string;
    }
}

class Joined implements String
{
    /**
     * @var String[]
     */
    private $strings;
    private $delimeter;

    public function __construct(array $strings, $delimeter)
    {
        $this->strings = $strings;
        $this->delimeter = $delimeter;
    }

    public function value()
    {
        return
            implode(
                $this->delimeter,
                array_map(
                    function (String $s) {
                        return $s->value();
                    },
                    $this->strings
                )
            );
    }
}

class UpperCase implements String
{
    private $string;

    public function __construct(String $string)
    {
        $this->string = $string;
    }

    public function value()
    {
        return strtoupper($this->string->value());
    }
}

var_dump(
    (new Joined(
        [
            new UpperCase(new DefaultString('harry')),
            new UpperCase(new DefaultString('potter')),
        ],
        ' '
    ))
        ->value()
);

It looks way more declarative, human-readable, composable and reusable.

 

There's no such assumption. Every issue I've listed holds true without it.

Eh, not really. I mean I get it... statics are incompatible with OOP or "objects all the way down" philosophy. Right on. But they have valid uses which remain testable and maintainable, albeit not following the OOP philosophy.

Anyway, not meant to argue your main point. I think you are right. Just that there are nuances to every tool. Best wishes!

 

You can think of a static class as a pseudo-namespace for functions. If your functions are free of state, side-effects and dependencies, there's absolutely nothing wrong with that - for one, functions are generally easier to write tests for.

Static classes in PHP are necessary, if you want to use functions, because literal functions can't be auto-loaded.

As long as you code according to the limitations of a real namespace, those classes are effectively just namespaces that can autoload.

That said, if I have functions in a library that someone might need to override or extend in a project, I go for a stateless service class instead, and sometimes an interface for abstraction - which enables dependency injection, so that someone can replace the service with their own project-specific implementation.

For example, something like "count unique occurrences of x in y" is a good candidate for something you'd never need/want to override, while something like "calculate shipping tax" is much more likely to have a project-specific definition.

Being unable to replace functions is the only "evil" thing about static classes, assuming it abides by the other mentioned constraints.

 

Yes, I agree, static classes are absolutely fine -- in procedural programming, but not in OOP.

 

So your argument is everything should be OOP? No one should use functions?

No, I'm just saying that static classes have nothing to do with OOP.

I think that's what I'm saying, by describing it as a pseudo-namespace.

But what's your point?

Because it sounds like you're trying to build an argument. Or perhaps you're just trying to clarify the fact that it's not automatically OO just because you use the class keyword?

The interface/class example wrapping a string made it sound dangerously like you think that would actually be meaningful somehow - I'm afraid you might mislead the people you're trying explain the difference to, towards actually programming like that.

My point is that static classes should not be used in OOP.

That example with decorated Strings is an OOP alternative to procedural StringUtils class.

It's okay to call functions though, like how your string example calls strtoupper and several others - but it's not okay to write/call your own functions?

I understand it's "not OOP", but "should not be used"?

I think there are well founded reasons why most languages support both paradigms - there are cases for functions and cases for classes, I think, and your string example is sufficiently complex and verbose as to leave me pretty firmly convinced of that.

 

Static methods can also be difficult to test, if they make use of dependencies that are environment-specific.

I would like to point out that the term "static class" has a specific, different meaning in Java. Java developers will usually assume that you are talking about a static nested class if you use that term.

Nice article!

 

Ah. In C#, static is a modifier you can put when declaring a class (top-level or nested). All it does is compile-time verify that all the class members are also static. Otherwise you will get an error. It doesn't have any particular usage implications that I'm aware of.