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

Route parameters passed to controller methods. #244

Open
danon opened this issue Aug 10, 2016 · 5 comments
Open

Route parameters passed to controller methods. #244

danon opened this issue Aug 10, 2016 · 5 comments
Labels

Comments

@danon
Copy link
Member

danon commented Aug 10, 2016

Routes

Route::get('/integration/:name/:id', 'controller#byNameAndId');

Controller

public function byNameAndId($name, $id) {
    $this->integration = Integration::findByNameId($name, $id);
}

instead of this

public function byNameAndId() {
    $this->integration = Integration::findByNameId($this->params['name'], $this->params['id']);
}

These parameters will always be present because routes would not point to those methods otherwise.
ReflectionParameter could be used to determine parameter name and assign proper param value.

@danon danon added the feature label Aug 10, 2016
@danon danon self-assigned this Aug 10, 2016
@bbankowski
Copy link
Member

Where do you want to use ReflectionParameter? I don't get it.

@danon
Copy link
Member Author

danon commented Aug 10, 2016

FrontController could lookup Controller's method's parameters' names, and assign them proper keys from $this->params.

So this could work

Route::get('/integration/:name', 'controller#byName');
public function byName($name) {
    $this->integration = Integration::findByName($name);
}

but this, would not:

public function byName($something) { // <-- exception, there's no such thing as $this->params['something']
    $this->integration = Integration::findByName($something);
}

@danon
Copy link
Member Author

danon commented Aug 10, 2016

For the second option to work you'd have to design routes like this

Route::get('/integration/:something', 'controller#byName');

@bbankowski
Copy link
Member

We could just use order of arguments, so no ReflectionParameter would be needed.

@piotrooo
Copy link
Member

piotrooo commented Aug 10, 2016

👍 for @bbankowski solution. Using order of arguments should have better performance. (Laravel uses this the way you described it 😄)

@danon danon closed this as completed Aug 11, 2016
@danon danon reopened this Aug 11, 2016
@danon danon removed their assignment Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants