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

Added getLoop method as protected #14

Closed
wants to merge 1 commit into from

Conversation

JordanRL
Copy link

@JordanRL JordanRL commented Feb 2, 2015

I recently ran into a problem where I wanted to extend the React\Socket\Connection class into my own Connection class, This meant also extending the server in order to replace the createConnection() method, however Connection objects require the event loop to be injected, and as the $loop variable is private, nothing extending the class can access it.

Instead of making the $loop variable protected, I opted to create a simple getter for $loop that can be used in extension.

I recently ran into a problem where I wanted to extend the React\Socket\Connection class into my own Connection class, This meant also extending the server in order to replace the createConnection() method, however Connection objects require the event loop to be injected, and as the $loop variable is private, nothing extending the class can access it.

Instead of making the $loop variable protected, I opted to create a simple getter for $loop that can be used in extension.
@staabm
Copy link

staabm commented Feb 2, 2015

You can catch the loop in your construct() and pass it then to parent::__construct()

@JordanRL
Copy link
Author

JordanRL commented Feb 2, 2015

Yes, but this is beside the fact that the Server class is fundamentally not built in a way that is open to extension.

@cboden
Copy link
Member

cboden commented May 6, 2015

I agree with @staabm. It's a much cleaner approach and accomplishes what you need; that by definition means it's open to extension.

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.

None yet

3 participants