Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kleinV2 can no longer provide custom methods #97

Closed
gbouthenot opened this issue May 22, 2013 · 9 comments
Closed

kleinV2 can no longer provide custom methods #97

gbouthenot opened this issue May 22, 2013 · 9 comments

Comments

@gbouthenot
Copy link
Contributor

It seems that since KleinV2, you can no longer provide custom methods to Response. The new ServiceProvider class behaves the same.

Currently I found 2 workarounds:
1/ define the methods in \Klein\App, but this is not as elegant.
2/ use $tmp = $rs->custom; $tmp(); This causes new problems as $rs is not available in the closure to call other functions.

This is a feature I really loves in KleinV1.

Also it is no longer possible to overwrite existing methods.

git checkout v1.1.0

<?php
require "klein.php";

respond(function($rq, $rs, $ap) {
    $rs->custom = function(){echo "CUSTOM METHOD\n";};
});

respond("/", function($rq, $rs, $ap) {
    $rs->custom();
});

dispatch("/");

->

CUSTOM METHOD

git checkout v2.0.1

<?php
require "vendor/autoload.php";

$k = new \Klein\Klein();

$k->respond(function($rq, $rs, $sr, $ap) {
    $rs->custom = function(){echo "CUSTOM METHOD\n";};
});

$k->respond("/", function($rq, $rs, $sr, $ap) {
    $rs->custom();
});

$rq = \Klein\Request::createFromGlobals();
$rq->server()->set("REQUEST_URI", "/");

$k->dispatch($rq);

->

PHP Fatal error:  Call to undefined method Klein\Response::custom() in /home/gilles/web/kleinv2/disa2.php on line 11
@Rican7
Copy link
Member

Rican7 commented May 22, 2013

Correct. The reason for this decision was simply that it felt a little dirty to be adding class-scoped methods at runtime.
The new design direction of Klein V2 was to move towards a more OOP style, in which you could easily add custom methods through dependency injection. For example:

Create a custom Response class that extends Klein's:

<?php

class CustomResponse extends \Klein\Response
{

    public function myCustomFunction($arg)
    {
        echo 'CUSTOM METHOD!';
        echo 'You passed: '. $arg;
    } 
}

And then inject it into the dispatcher:

<?php
require 'vendor/autoload.php';

$k = new \Klein\Klein();

$k->respond('/', function($rq, $rs, $sr, $ap) {
    $rs->myCustomFunction('woot!');
});

$rq = \Klein\Request::createFromGlobals();
$rq->server()->set('REQUEST_URI', '/');

$rs = new CustomResponse();

$k->dispatch($rq, $rs);

Does that make sense?

@gbouthenot
Copy link
Contributor Author

Thank you for your wuick answer !

This is relevant, but this raise new problems related to multiple inheritance. Traits would probably be a better solution I think, but the're not supported with php 5.3.

I still find the kleinV1 syntax way more concise and elegant. I will try to find new ways to work around this limitation in kleinV1.

@Rican7
Copy link
Member

Rican7 commented May 22, 2013

No problem!

but this raise new problems related to multiple inheritance

How so? Multiple inheritance is a problem when trying to extend from multiple classes horizontally. Its no problem in a vertical chain. Simply extending a class to "extend" its functionality is a very common practice in PHP and all other object oriented languages. 👍

Traits would probably be a better solution I think, but the're not supported with php 5.3.

Traits are designed for a more horizontal inheritance pattern. They aren't required for this type of functionality extension.

From the PHP docs:

"A Trait is similar to a class, but only intended to group functionality in a fine-grained and consistent way. It is not possible to instantiate a Trait on its own. It is an addition to traditional inheritance and enables horizontal composition of behavior; that is, the application of class members without requiring inheritance."

I still find the kleinV1 syntax way more concise and elegant

Klein V1 was beautiful! It was elegant, simple, etc. Unfortunately, though, it used a lot of magic to make PHP classes/objects behave in a way that allowed for hard to document/maintain code. This new, more explicit approach is an attempt to fix that behavior and build off of the direction that Klein was already heading in version 1 (new classes were added just to create headers, etc).

@gbouthenot
Copy link
Contributor Author

In your example, you use

class CustomResponse extends \Klein\Response

If there are more than one source of method defining, you'll get another

class AnothercustomResponse extends \Klein\Response
// (or)
class AnothercustomResponse extends CustomResponse

Anyway, I am not satisfied with this solution, because I want to be able to have many method injections.

I will soon send a pull request to restore this behaviour. Hope you will accept it.
(due in a few hours)

@Rican7
Copy link
Member

Rican7 commented May 23, 2013

What do you think about this, @chriso?
I think its providing awkward magic and fights the design principles of the object oriented structure. Even Sinatra (kleins original inspiration) uses an object oriented class inheritance model for extending Sinatra's functionality (through helpers), and Sinatra's a more "functional" (a little less OOP structured in use, not design) micro-framework.

Also, I still don't get what you're trying to say here, @gbouthenot. Extending classes to add OO functionality is the most logical and "classical" way of achieving your goal here. Your solution would make more sense in a "prototypical" inheritance model.

@gbouthenot
Copy link
Contributor Author

I see your point. What I am trying to say is that kleinV2 broke this feature I used a lot. I think that the V2 should not purposely remove features, when the changes to get the features back are minimal.

@Rican7
Copy link
Member

Rican7 commented May 23, 2013

Yea, I understand that, but V2 wasn't meant to be backwards compatible, honestly. Its a totally different control flow, methods were moved and renamed, its no longer in global scope, etc.

Also, I originally had a similar magic __call() method in Klein V2, but removed it per @chriso's request (see 14).

@chriso
Copy link
Contributor

chriso commented May 24, 2013

Klein v1 opted for mixins rather than OO. This is great if you want to get an app up and running quickly (one of my original goals - I just needed to bang out admin scripts quickly) but you'll eventually run into trouble. What if a user wants to override or extend one of the built-in request/response methods? Inheritance is the way to go.

@gbouthenot all of your klein v1 mixins are dispatched (globally) to each route so I'm not sure why multiple inheritance was brought up.

@Rican7 my vote is to keep the magic to a minimum and to not worry about backwards compatibility with v2.

@chriso chriso closed this as completed May 24, 2013
@Rican7
Copy link
Member

Rican7 commented May 24, 2013

@chriso Thanks Chris!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants