1

I am doing a class "Container" to hold all my model/service instances, the class is a singleton.

Consider the following code portion (from a CodeIgniter project):

public function getReviewModel()
{
    static $loaded = false;

    if (!$loaded)
    {
        $this->load->model('review_model');

        $loaded = true;
    }

    return $this->review_model;
}

I am wondering if is still ok to use static inside method like this or should I use only class property (I mean about performance and coding standard) ?

Thomas Decaux
  • 21,738
  • 2
  • 113
  • 124

2 Answers2

2

In your example, nothing prevents the programmer to instanciate the class more than once (with dire results), so it is rather a confusing bit of code.

Static variables have their uses, be them local to a function or defined at class level.
Especially in PHP scripts, when the output is often a single piece of data that can conveniently be handled as a class defining only static properties and methods.
That would be a true, foolproof singleton class.

Since mistaking a static variable for a dynamic one is a common pitfall, I tend to favor static class variables to avoid the confusion (i.e. the self::$... syntax makes them stand out clearly).

kuroi neko
  • 8,479
  • 1
  • 19
  • 43
1

General consensus as far as statics are concerned in PHP is: Avoid, if at all possible. And yes, 99% of the time, it is possible to avoid statics.
Singletons should be avoided 100% of the time. For reasons you can find here and virtually everywhere else on the web. Singletons are like communism: sounds like a nice idea, but when put in to practice, it turns out there's one or two things you didn't anticipate.
A Singletons' main purpouse is to retain state, but PHP itself is stateless, so come the next request, the singleton needs to be re-initialized anyway.

If I write getters like yours, I tend to create them in a lazy-load kind of way:

class Example
{
    private $reviewModel = null;//create property
    public function getReviewModel()
    {
        if ($this->reviewModel === null)
        {//load once the method is called, the first time
            $this->reviewModel = $this->load->model('review_model');
        }
        return $this->reviewModel;
    }
}

This basically does the same thing, without using statics. Because I'm using a property, I still retain the instance, so if the getReviewModel method is called again, the load->model call is skipped, just as it would be using a static.
However, since you're asking about performance as well as coding standards: statics are marginally slower than instance properties: Each instance has a HashTable containing for its properties, and a pointer to its definition. Statics reside in the latter, because they are shared by all instances, therefore a static property requires extra lookup work:

instance -> HashTable -> property
instance -> definition -> HashTable -> property

This isn't the full story, check answer + links here, but basically: the route to a static propery is longer.

As for coding standards: They exist, though still unofficial, most major players subscribe to them, and so should you: PHP-FIG
Things like $this->_protectedProperty; don't comply with the PSR-2 standard, for example, which states, quite unequivocally:

Property names SHOULD NOT be prefixed with a single underscore to indicate protected or private visibility.

Community
  • 1
  • 1
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
  • Where do you see we should avoid statics inside method? I am looking for the information actually, because I always thought its better (in this particular case) than create X class properties, thanks. – Thomas Decaux Feb 03 '14 at 09:20
  • @ThomasDecaux: why singletons are bad, and why statics in PHP make little to no sense has been covered many times on this site [but here's a nice example](http://stackoverflow.com/a/4596323/1230836) of one of the answers. Basically: statics are globals in drag, that impede testing above all else. PHP is, in essence a stateless runtime. Why, then, use a pattern that is all about persisting state? Just look at the following three words, which doesn't match the other 2: Stateles, OO and static (bare in mind that statics are globally available, too). – Elias Van Ootegem Feb 03 '14 at 09:35
  • Sorry when I mean singleton, I mean a single instance of Container, not the "singleton pattern". I just want to know if there is a good reason to use class property instead of static variable within class methods. – Thomas Decaux Feb 03 '14 at 10:01
  • Thanks for the memory management explanation, very detailed. But if I store 100 private class properties never set, does it takes "memory"? – Thomas Decaux Feb 03 '14 at 10:03
  • @ThomasDecaux: Yes: A property is slightly more performant, makes your code easier to test, and more reusable, static-driven code doesn't scale, is harder to test, and make no sense in PHP. If you need access to a proprty, using static calls is just an ugly quick-fix – Elias Van Ootegem Feb 03 '14 at 10:03
  • @ThomasDecaux: In a way, yes: it takes -depending on your system- anything rangind from 800bytes to 1.6 k in memory (1 property => 1 offset in `HashTable` + 1 `zval` pointer... nothing to be worried about, or even care about. Other things take up 10x more memory, besides, this memory is allocated/freed along with your instance. Statics are allocated globally, so stay in memory longer – Elias Van Ootegem Feb 03 '14 at 10:06
  • Okay ! This sound a very good answer, thanks to have took some time for me. Where can I find good resource about PHP and memory? I will use sub-containers in order to split my big container. – Thomas Decaux Feb 03 '14 at 10:11
  • @ThomasDecaux: Well, if you want to learn about PHP's internals, you can browse the source (if your C is proficient), or you can use the [phpinternals](http://www.phpinternalsbook.com/) site – Elias Van Ootegem Feb 03 '14 at 10:34
  • Voting this down as I don't really see how, in this case, a static var would be bad for tests. It's essentially internal to the method! It **IS** used to keep state - requests might be stateless, but within a request you can surely have state needs. Besides that, your suggested instance prop (even when private) is simply exposing the internals of that method to the outside, and can cause confusion for developers - _"when should I use the prop and when should I use the method?"_. A static var inside the method erases the issue while keeping state fast, using previously generated values. – igorsantos07 Feb 21 '19 at 05:44