0

Through my multiple studies I have come across the factory method of setting session and database objects which I have been using while in development. What I am wondering is, putting aside personal preference (although I will soak in any opinions anyone has), does this general method work, and is it efficient (meaning, am I using it correctly)? If it is not, do you have suggestions for how to improve it?

Background

I created the code this way so as to pass a database and session object to the class upon calling the class. I wanted to be able to pass along the relevant objects/references so that they could be used.

The Call Class

This class is meant to call static functions, like so:

class CALL {
    public static $_db, $_session;

    public status function class1() {
        $function = new class1();
        $function->set_session(self::$_session);
        $function->set_database(self::$_db);
        return $function;
    }

    public status function class2() {
        ...
    }
    ...
 }

The _set class

class _set {
    public $_db, $_session;

    public function __construct() { ... }

    public function set_database($_db) {
        $this->_db = $_db;
    }

    public function set_session($_session) {
         $this->_session = $_session;
    }
}

Now the classes referenced.

class class1 extends _set {
    function __construct() { ... }

    function function1() { return "foo"; }
    ...
}

So, moving forward, the classes would be called using CALL::class1 or CALL::class2. After that, they can be accessed as per usual, aka:

CALL::$_db = $database->_dbObject;
CALL::$_session = $_SESSION;
$class1 = CALL::class1;
echo $class1->function1(); //prints "foo".
smcjones
  • 5,490
  • 1
  • 23
  • 39
  • 1
    Well .. you could start by getting rid of the global state. That would definitely be a improvement. Maybe [this](http://stackoverflow.com/a/11369679/727208) gives you some idea. – tereško Dec 09 '13 at 06:35
  • Wow, awesome example and exactly what I needed. I put a +1 on that answer and would have marked it correct for my question were it here. Thanks! – smcjones Dec 09 '13 at 11:46

2 Answers2

1

Read about Dependency Injection . Small suggestion from my point of view, you should never create objects like $db or $session inside other objects. You should rather inject them through constructor or setter method. It will make your code less dependant on a specific classes and it will be easier to replace all dependencies almost without refactoring (actually without one if you know hot to use interfaces).

pawel.kalisz
  • 1,246
  • 1
  • 18
  • 26
  • 1
    He is already using dependency injection as an approach. It is **not mandatory** to pass all dependencies in a constructor. – tereško Dec 09 '13 at 06:36
  • Yes you are right, this is some kind of DI, but it is clumsy one from my point of view. If take a look at last code block there is a line `$class1 = CALL::class1;` which in my opinion should looks like `$class1 = CALL::class1($db, $session);`. Code is more readable and more flexible, because you don't have to change CALL class implementation to change $db class or $session class. I see that he uses static fields in first two lines but for me it is a poor design, and as you mentioned above such global state is not the right approach either. – pawel.kalisz Dec 09 '13 at 06:42
  • +1 for "use constructor or setter methods" and for mentioning interfaces. Will post an update on what I end up doing but will not be able to change it until this evening. This definitely points me in the right direction and may well end up being listed as "correct". – smcjones Dec 09 '13 at 11:40
  • I would be interested to hear your thoughts on this tutorial, as it is what I based my code off of. I do realize I need to change the properties to private static.. http://www.potstuck.com/2009/01/08/php-dependency-injection/ – smcjones Dec 09 '13 at 18:43
  • After careful thought, I have decided this is a very unworkable solution for me. The whole point of having a factory or Container class is to not have to repeat code. When one has to use my CALL Container class to call a function, and each time I call, I need to pass along database and session objects, I am going to be in a world of hurt if I need to change those two values. Search and replace is not the best tool for this job. – smcjones Dec 12 '13 at 00:39
0

If anyone stumbles on this, I will share with you what my solution was.

Although this exercise helped me to learn a lot, and I am sure I could take the time to create a VERY highly functional factory/Container, because this is not integral to my program and not even unique, I finally bowed to the age old wisdom of not repeating something that has already been done.

I utilized Pimple, a lightweight library that uses PHP closures to create function calls. Now, I can haave the flexibility of determining which dependency injections I want, but I also only need to inject them once. Future calls, even when they create new instances, will replicate them. While I think that, in theory, my project was workable as it was, it did indeed have the unfortunate issue of requiring you to go into the container to make changes. With Pimple I do not need to do that. So I've tossed by Container class and picked up a lightweight program from the maker of Symfony. While this may not be the best answer for everyone, it was for me. Cheers!

smcjones
  • 5,490
  • 1
  • 23
  • 39