PHP OOP :: getting confused about Factory class and passing objects to constructors

Question

I am trying ensure that my app enforces Dependency Injection but I'm getting into a bit of a pickle...

class Factory {

public $sessionId;
public $config;
public $lib;

function __construct() {
    $this->getSessionId();
}

function getSessionId() {
    if (isset($_COOKIE['sk'])) {
        $this->sessionId = trim($_COOKIE['sk']);
    } else {
        $sm = $this->createSessionManager();
        $this->sessionId = trim($sm->getNewKey());
        setcookie("sk", $this->sessionId, time() + 3600 * 3, "/");
        setcookie("vb", 0, 0, "/");
    }

}

function createBasket() {
    $t = $this->createTicket();
    $co = $this->createContribution();
    $cfg = $this->createConfig();
    $basket = new Basket($t, $co, $cfg);
    return $basket;
}

function createTicket() {
     $sm = $this->createSessionManager();
    $cfg = $this->createConfig();
    $ticket = new Ticket($this->sessionId, $sm, $cfg);
    return $ticket;
}

.... }

First of all, I'd like to know if I'm approach thing in the right way. An example viewer page - called by a browser - would be:

function __autoload($class_name) {
    include 'classes/' . $class_name . '.class.php';
}

$factory = new Factory;

$performance = $factory->createPerformance();
$pno = (isset($_GET['pno']) ? $_GET['pno'] : 0);
print $performance->displayHtmlListing($pno);

My other question/problem is related to how I prevent a 'catch 22' situation where objectA requires objectB that - in rare circumstances - will need objectA.

An example, in Factory class:

function createParser() {
    $am = $this->createApiManager();
    $parser = new Parser($am);
    return $parser;
}

The parser object is passed an object into constructor to satisfy its dependencies in the business logic. Its job is to read requests for files and if a certain condition is met, it needs to grab hold of the $am object (APIManager) and fire off a request to the API. The problem is that APIManager passes all requests through the Parser object, and I can't easily remove the dependency on Parser without breaking business rules or adding redundant conditional code.

Wondering if anyone had some thoughts on how to get around this issue?

Thanks, James

UPDATE:

Bit more info on the dependency problem:

ApiManager.caller() - it's job is to go off to an API URL send GET variables, and retrieve results.

This caller() method is used by nearly all classes (perhaps a good reason for it to be in super class and extend the classes that need it?).

Part of the caller() responsibility is to check what API calls are being made. If a certain condition is met, it needs to stop and make a call to SessionManager.getNewSession() which goes off to the same API and retrieve a value. Only when this is complete can the original request ( in caller() ) be completed.

The problem is that SessionManager.getNewSession() also uses caller().

Answer 1

This is only IMHO, but having a large proportion of classes with names containing "manager", "context", "parser", etc. is a code smell. I'm not saying these kinds of classes are necessarily wrong -- certainly virtually every app has them. But think about it -- is a parser a thing you have, or is parsing a thing that an object does? Is there a real-world guy with the title of "API Manager"? Does he hang out with the Session Manager and ask you about your TPS reports? Etc.

Think about your class design - I'm not saying it's "wrong" as it is -- because I haven't seen it and it's subjective anyway -- but are your classes modeling real-world entities, or are they modeling actions that can be carried out on entities? Or, even worse, are they modeling some kind of arbitrary workflow? Do your classes all have clearly defined responsibilities and not try to do crazy stuff that's none of their business?

Basically, you're running into this problem because your class design is not optimal. In particular, if an instance of one class requires an instance of another class and vice-versa to work properly, you would probably benefit from rethinking your design.

Update in response to James' update

ApiManager.caller() - it's job is to go off to an API URL send GET variables, and retrieve results.

One of the ideas of OOP is that classes represent entities (nouns) and methods represent actions (verbs). ApiManager here is an entity (kind of an abstract one, but we'll go with it) but caller() is not an action - i.e. you can't "caller" something. You could however call() something -- I guess in the context of ApiManager, this would "call" the API. I know it's only 2 little letters, but it makes a big difference in one's mental concept of what the ApiManager does.

This caller() method is used by nearly all classes (perhaps a good reason for it to be in super class and extend the classes that need it?).

Inheritance best used to support polymorphism, not code sharing -- I wouldn't do that...

Part of the caller() responsibility is to check what API calls are being made. If a certain condition is met, it needs to stop and make a call to SessionManager.getNewSession() which goes off to the same API and retrieve a value. Only when this is complete can the original request ( in caller() ) be completed.

One problem is that caller() shouldn't have a responsibility because it should be an action (e.g. call()), not a thing. Actions can't have responsibilities. It should just do something, preferably something simple. In keeping with the idea of ApiManager as a thing and call() as an action that ApiManager does, here is an example of the very bare minimum of how I might expect an ApiManager to work:

class ApiManager {

    private $session;
    private $url;

    // You'll probably need to set up the object differently;
    // this is just an example....
    public function __construct($url) {
        $this->setURL($url);
    }

    function call() {
        if ($this->session is expired) {
            // instead of calling SessionManager.getNewSession() here,
            // why not just fold the session management functionality
            // into ApiManager? They both use the same API, after all...
            $session = $this->getNewSession();
        }
        // do your call & return the response
    }

    public function setURL($url) {
        $this->url = $url;
    }

    private function getNewSession() {
        // get a new session
        $this->session = $my_new_session;
    }

}

You'll notice that all the methods in the class above are actions, they all do something. The icing on the cake is that there is no longer any need to worry about dependency injection for SessionManager, because it no longer exists! After all, an API session is really something that only the ApiManager needs to know about, no? :)