Factories

[geggleto]

I would love to see proper Factories.

Will PR @akrabat ;)

[codeguy]

We can probably deprecate/remove the entire "environment" concept in favor of these new factory methods.

[codeguy]

Can you provide a code snippet here that demonstrates what you want? I'm thinking something like \Slim\Http\Request::createFromGlobals($_SERVER);. Seem fine?

[geggleto]

I was thinking....

class RequestFactory {
   public function __construct(BodyFactory $bodyFactory);

   // returns \Slim\Http\Request
   public function build($attributes, $headers);
}

$request = $requestFactory->build($_SERVER, $headers);

The reason behind it, so we have more control of the Body Object and how it's created. The Request shouldn't know or care how the Body is created, or URI object.

[codeguy]

👍 Will get started today.

[codeguy]

@geggleto Couldn't the same be said for headers and uri?

[codeguy]

I'm not entirely sold on the need for separate factory classes... a class can know how to make itself, so why not Body::createFromText|Stream(), Uri::createFromGlobals(), Header::createFromGlobals(), and Request::createFromGlobals()? We can always supply a default body (from raw text or a stream resource), and it can easily be overridden with $request->withBody().

[codeguy]

I'm just worried about over-engineering for engineering's sake.

[geggleto]

For me it's abut reducing the class complexity. Factories are clean and easy to read. For someonek like me who does read the slim source it is nice to have readable code.

I would say choose whatever is readable and testable. Decoupling away from the use of globals inside methods and surfacing them to the signatures is the ultimate goal.

/cc @codeguy

[codeguy]

Cool. I've started toying with an abstract factory. I've added the FactoryInterface interface and the FactoryDefault implementation. This will let us recycle the same creation logic in our tests and in the actual codebase. PHP 7 syntax for now while I'm playing around with some ideas.

[geggleto]

PHP 7 <3. I like the abstract factory pattern and this is looking quite good.

[geggleto]

https://gist.github.com/geggleto/37539ebbd679f28995b4

Been toying around with moving the construction logic.

I really want to rename the Environment class as its a container around _SERVER.

@akrabat @codeguy

[geggleto]

I think we should rename the Request class, as in PSR 7 there are differences between ServerRequest and just Request. Slim internally uses ServerRequestInterface, so it would probably be best to reflect that.

[geggleto]

Dropping this in here,

https://github.com/geggleto/slim-http

Lot's of refactoring, and renaming.

[akrabat]

If we support factories, then https://github.com/http-interop/http-factory is the way to do it.