klein / klein.php

A fast & flexible router
MIT License
2.66k stars 290 forks source link

Can we reach $app/$req from a class scope? #276

Closed Aristona closed 9 years ago

Aristona commented 9 years ago

I have a code like this:

$klein->with('/tests/?', function () use ($klein) {

    $klein->respond('GET', 'test', function($req, $res, $service, $app) {

        $retriever = new \Aristona\RSS\Retrieve\Retriever();
        $retriever->retrieve();
    });

});

I need to access $app and $req inside Retriever class. How can I do it? I don't want to pass every variable into my class via constructor.

jk3us commented 9 years ago

For $app, you may want to look at registering a service to act as a dependency injector... So, before that you could have

$klein->respond(function ($request, $response, $service, $app) {
    $app->register("Retriever", function() use ($app) {
        return new \Aristona\RSS\Retrieve\Retriever($app);
    }
}

$klein->with('/tests/?', function () use ($klein) {
    $klein->respond('GET', 'test', function($req, $res, $service, $app) {
        $app->Retriever->retrieve($req);
    });
}

I like to pass in exactly what I need instead of the the whole $app or $request objects. It's a good idea for your Retriever class to not depend too strongly on klein.

Rican7 commented 9 years ago

The Klein instance properties can't be accessed from a "class" scope, the instances have to be passed around. If you need to access the $app and/or $req inside your Retriever class, you'll need to pass them in... however I wouldn't recommend doing that either.

As @jk3us put it, having your RSS Retriever class depend on the Klein concretion isn't the best design, it tightly couples your class with Klein's concretions. Instead, I'd recommend that your Retriever class take the smaller and more abstract pieces that it needs (instead of the whole $app context, for example) and maybe take in just the string body of the request or the parameters, instead of the entire request object itself.

Aristona commented 9 years ago

Hi,

Thanks for the replies. I had to access both $req->articleID and $this->app->articleDB->add(...) on the class scope. The best practice is simply passing $this->app->articleDB and $req->articleID via constructor?

Rican7 commented 9 years ago

It depends on your Retrieval class' structure. If you always need the DB for that class, I'd pass the articleDB into the constructor. However, since you'll probably want to allow the class instance to retrieve more than just one article, I'd pass the articleID into the retrieve() method. Something like this, maybe:

$klein->respond(function ($request, $response, $service, $app) {
    $app->register("Retriever", function() use ($app) {
        return new Aristona\RSS\Retrieve\Retriever($app->articleDB);
    }
}

$klein->with('/tests/?', function () use ($klein) {
    $klein->respond('GET', 'test', function($req, $res, $service, $app) {
        $app->Retriever->retrieve($req->articleID);
    });
}

Again, this is just taking a guess at your structure. You'll need to decide what your class needs to do based on your application.