Skip to content
This repository has been archived by the owner on Sep 19, 2022. It is now read-only.

Better RESTful routing #19

Merged
merged 1 commit into from
Jan 8, 2014
Merged

Better RESTful routing #19

merged 1 commit into from
Jan 8, 2014

Conversation

yaworsw
Copy link
Contributor

@yaworsw yaworsw commented Jan 8, 2014

Changed how http methods are defined in the router. Http methods are now defined like this:

$app->addRoutes(array(
    '/'               => 'Home:index',
    'get /posts'      => 'Posts:index',
    'get /posts/:id'  => 'Posts:show',
    'post /posts'     => array('Posts:create', function() { echo 'Middleware!'; }),
    ...
));

@yaworsw
Copy link
Contributor Author

yaworsw commented Jan 8, 2014

See #17

I will merge this into the develop branch since it is a breaking change. To use this functionality use dev-develop (until version 0.4.0 is released) as your version in composer.json.

yaworsw added a commit that referenced this pull request Jan 8, 2014
@yaworsw yaworsw merged commit e0109b9 into fortrabbit:develop Jan 8, 2014
@falmp
Copy link
Contributor

falmp commented Jan 30, 2014

I know this might be too late - and honestly even me is not convinced about this alternative syntax, so I'm just putting this here to raise a discussion -, but what do you think about this?

$app->addRoutes(array(
    '/posts' => array('GET'  => 'Posts:index',
                      'POST' => 'Posts:create')
));

@yaworsw
Copy link
Contributor Author

yaworsw commented Jan 30, 2014

No I wouldn't say its too late. This is exactly why I didn't merge the alternate syntax into master (I should have maybe done an RC release... and not closed the discussion but whatever).

I went and made some example routes using both syntaxes.

$requireAuthentication = new Middleware\RequireAuthentication();

array(
  '/me'                        => array('get'    => array('User:me',       $requireAuthentication),
                                        'post'   => array('User:update',   $requireAuthentication),),
  '/user/:username'            => array('get'    => array('User:show'),),
  '/user/:username/follow'     => array('post'   => array('User:follow',   $requireAuthentication),
                                        'delete' => array('User:unfollow', $requireAuthentication),),
  '/:username/tweet/:tweet_id' => array('get'    => array('Tweet:show'),),
);

array(
  'get    /me'                        => array('User:me',       $requireAuthentication),
  'post   /me'                        => array('User:update',   $requireAuthentication),
  'get    /user/:username'            => array('User:show'),
  'post   /user/:username/follow'     => array('User:follow',   $requireAuthentication),
  'delete /user/:username/follow'     => array('User:unfollow', $requireAuthentication),
  'get    /:username/tweet/:tweet_id' => array('Tweet:show'),
);

Could I have done the first set in a more readable way? Also seeing them both side by side what do you think?

@yaworsw
Copy link
Contributor Author

yaworsw commented Jan 30, 2014

I'll create a new open issue where it can be discussed further later on today.

@yaworsw yaworsw mentioned this pull request Feb 1, 2014
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants