3
\$\begingroup\$

I'm currently writing a REST-tool that connects to a third-party server (duh). They offer a fairly large (if not too large) API in a single wsdl file, as well as separate wsdl's for each specific part of data you want to work on.
When writing my code, I created an abstract Api class, and a number of child classes, one for each subset of the API.

Since all requests are available through a single, "master-api", there is only 1 user-password required to connect, so I've created the getClient and setClient methods in the abstract class.
Since it's not unlikely form me to be working with two, or three of these API-classes, I decided to create a private static property in the abstract class. This is to avoid excessive internal method calls, brought along by lazy-loading.
Is this a valid use-case for the static keyword? It's the only time I'm using static in my entire code, and I have this acquired repulsion for using it.

Anyway, here's some (grossly oversimplified) version of my code:

namespace MyLib;
use Tool\Application,
    Tool\Command;//it's a CLI tool
use MyLib\Soap\Client;
use \Zend_Config;//can't help but liking the old Zend_Config

absctract class Api
{
    //all properties have a getter, which lazy-loads the correct object, where possible
    //can be injected through constructor, setter or via $this->command
    private $app = null;
    //inject via setter, get from application or command
    private $config = null;
    //inject via setter or child constructor
    private $command = null;
    /* This is the one */
    private static $login;

    public function __construct(Application $app = null)
    {
        $this->setApplication($app);
        if (!is_callable(array($this, 'init')))
        {
            throw new \RuntimeException('Impossible, yet somehow '.get_class($this).'::init() cannot be called');
        }
        return $this->init();
    }

    abstract protected functoin init();

    final private function getLogin()
    {
        if (self::$login === null)
        {
            $config = $this->getConfig();
            $this->setLogin(array(
                array(
                    'login'   => $config->api->login,
                    'pwd'     => $config->api->pwd
                )
            );
        }
        return self::$login;
    }

    final private function setLogin(array $login)
    {
        if (count(array_filter($login) === 2)
        {
            self::$login = $login;
        }
        return $this;
    }

    protected fucntion getClient()
    {
        if ($this->client === null)
        {
            $wsdl = $this->getConfig();
            $wsdl->api->{$this->getName()}->wsdl;
            $this->setClient(
                new Client(
                    $wsdl,
                    $this->getLogin()
                )
            );
        }
        return $this->client;
    }

    public fucntion setClient(Client $client = null)
    {
        if ($this->client)
        {
            $this->client->disconnect();
        }
        $this->client = $client;
        return $this;
    }

    protected function getConfig()
    {
        if ($this->config === null)
        {//getApplication falls back to $this->getCommand()->getApplication()
            $this->setConfig(
                $this->getApplication()->getConfig()
            );
        }
        return $this->config;
    }

    public function setConfig(Zend_Config $config = null)
    {
        $this->config = $config;
        return $this;
    }
}

As for the child classes, they look something like this:

namespace MyLib\Apis;
use MyLib\Api;
use MyLib\Data\Models\Model;

class Foo extends Api
{
    public function __construct($mixed = null)
    {
        if ($mixed instanceof Command)
        {
            $this->setCommand($mixed);
        }
        parent::__construct($mixed instanceof Application ? $mixed : null);
    }

    protected function init()
    {
        $this->setName('foo');//<-- init is a tad more complicated, but not relevant here
    }

    public function doRequest(Model $data = null)
    {
        $client = $this->getClient();
        //worst case here:
        // parent::getClient getConfig -> getApplication -> getCommand {->setApplication(command->getApplication())}->getConfig <== setConfig of $this here
        return $client->getSomething(
            array(
                'param'  => $data->foobar
            )
        );
    }
}

Every getClient can result in as much as 12 method calls. By creating a static array containing the login data, I can make sure that the maximum number of calls is 12 once, and then, a maximum of 9 calls applies to all instances.
If this is indeed a good use case for statics, I'm thinking of creating a second static (though my heart really is bleeding just thinking about it) that'll hold the Application instance, which, in order for an application to use my lib, it needs to hold the config file, which is what I'm after.

If I create those two statics, the theoretical maximum of 12 method calls remains, but after the full 12 calls have been made once, This can be reduced to only 4 when I'm getting the config, 6 upon calling getClient the first time.
Half of the calls I'm currently making, which, could make a difference IMO

\$\endgroup\$
2
\$\begingroup\$

Is it a "good" use-case? Not in my opinion. Is it "decent"? I think it is decent enough to get the job done if that is how you want to do it. The beauty of it is, anything is "decent" enough if it works for you, and things could always be "better". I find that I mostly use statics for Singletons and simple utility functions -- otherwise, the benefits rarely outweigh the drawbacks.

Personally, I would not go this route; even though it may make things easier to write initially, it would also make it hard to do any type of unit testing.

It seems like you may also be assuming that no one, ever, for any reason, may have more than one set of login details that they would like to use at the same time. While that might be true, I wouldn't want to make that assumption.

If it were me, I would approach this like any other adapter -- create independent classes with single responsibilities and use dependency injection for configuration/etc. Finally, I would create a facade which implements those responsibilities, and handles the dependencies, in order to provide an "easy to use" interface without taking away the user's ability to do things at a lower level.

To me, this would give you the most testability and maintainability, without placing constraints on the how someone else could use the code (and still provide an easy interface as well).

Of course, that is just my opinion.

\$\endgroup\$
  • \$\begingroup\$ Well, the use of a static $login might indeed prove to be a drawback in time, but a static $app? These are "services" or "helpers", that the application will be using in order for it to get data via the API. The Application cannot run without having the $config set, and I can inject the application instance once, set it statically and use it everywhere, can't I. That way, if multiple logins come into play, there still is no issue \$\endgroup\$ – Elias Van Ootegem Jul 4 '13 at 10:31
  • \$\begingroup\$ PS: You talk about dependency injection, but at the same time admit to using Singletons in PHP... Sorry if I'm being obtuse here, but that doesn't make sense IMO \$\endgroup\$ – Elias Van Ootegem Jul 4 '13 at 11:46
  • \$\begingroup\$ @EliasVanOotegem - To your first comment -- yes and no, in my opinion. While you "can" inject everything into a single container which everything references, this leaves no flexibility without complicating your classes. Take, for example, a second configuration or login -- this would require you to now add special code to your classes for determining which "set" of information to use. It would be better to allow this information to be passed, which abstracts away the environmental "choices" and allows the classes to focus only on accomplishing their tasks. (continued...) \$\endgroup\$ – Jacob S Jul 8 '13 at 14:04
  • \$\begingroup\$ I know you're probably typing the rest of your reply ATM, but I should point out that the config object is structured in such a way that each component has only accesses a subset of the entire config, based on the abstract class (using its namespace, instance name and task: app.helpers.db.user_db.login="userlogin" and so on) so adding new login data for a specific class is doddle, the constructor and init functions take care of that for me \$\endgroup\$ – Elias Van Ootegem Jul 8 '13 at 14:07
  • 1
    \$\begingroup\$ To me, there is also nothing wrong with having a $app to accomplish bootstrapping and create a facade for your underlying structures. The difference is in passing the $app to your individual components and having them make their own choices as to what to do with this larger structure, rather than limiting their responsibilities to solely accomplishing the tasks for which they are intended. Now -- maybe you are using additional patterns within your $app which limit this, but I'm just giving my opinion on why I wouldn't approach it that way. \$\endgroup\$ – Jacob S Jul 8 '13 at 14:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.