Skip to content

Increase method visibility#27

Open
skyosev wants to merge 1 commit intomrjgreen:masterfrom
skyosev:patch-1
Open

Increase method visibility#27
skyosev wants to merge 1 commit intomrjgreen:masterfrom
skyosev:patch-1

Conversation

@skyosev
Copy link

@skyosev skyosev commented Jul 14, 2015

I really don't see a reason why the method of this class should be private. There are some realistic cases for extending the class and overriding some functionalities. For example I would want to throw events at some points from the dispatch cycle. The only solutions for me are to replace the whole class (pretty ugly) or modify the source code directly (renders composer useless).

Best Regards

I really don't see a reason why the method of this class should be private. There are some realistic cases for extending the class and overriding some functionalities. For example I would want to throw events at some points from the dispatch cycle. The only solutions for me are to replace the whole class (pretty ugly) or modify the source code directly (renders composer useless). 

Best Regards
@mrjgreen
Copy link
Owner

Hi! Thanks for the feedback. I could make this protected. I'll think about it over the weekend. I feel the correct solution is to break this part of the class out into a handler object which can be injected in. Then you could write your own handler which implements a handler interface, instead of using inheritance. That may be a breaking change so I will look at trying to do that for the next version.

https://github.com/mrjgreen/phroute/tree/handler/src/Phroute

I actually have a partially complete branch which attempts to do this, but I had to make too many BC breaks to make it work. I'll try and find a way of doing this thats a little less destructive :)

@frederikbosch
Copy link
Contributor

@skyosev Why don't you use composition inside your code? So you inject the Phroute Dispatcher into your own created Dispatcher? And then you decorate the Phroute dispatcher with your own methods!

@kamalkhan
Copy link

Hi, could you also pass the variables ($var) onto the resolver resolve method? Right now I will have to fork the project and modify the Dispatcher class to send the $var as the second parameter to my resolver method. As @skyosev has mentioned about the visibility, I wouldn't have asked if the class had protected methods instead of private.

Thanks!

@W1zzardTPU
Copy link

+1 on merging this to avoid forking the whole project

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 this pull request may close these issues.

5 participants